2022-05-09 13:49:05

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 00/10] Coresight: Add support for TPDM and TPDA

This series adds support for the trace performance monitoring and
diagnostics hardware (TPDM and TPDA). It is composed of two major
elements.
a) Changes for original coresight framework to support for TPDM and TPDA.
b) Add driver code for TPDM and TPDA.

Introduction of changes for original coresight framework
Support TPDM as new coresight source.
Since only STM and ETM are supported as coresight source originally.
TPDM is a newly added coresight source. We need to change
the original way of saving coresight path to support more types source
for coresight driver.
The following patch is to add support more coresight sources.
coresight: core: Use IDR for non-cpu bound sources' paths.

Introduction of TPDM and TPDA
TPDM - The trace performance monitoring and diagnostics monitor or TPDM in
short serves as data collection component for various dataset types
specified in the QPMDA(Qualcomm performance monitoring and diagnostics
architecture) spec. The primary use case of the TPDM is to collect data
from different data sources and send it to a TPDA for packetization,
timestamping and funneling.
Coresight: Add coresight TPDM source driver
dt-bindings: arm: Adds CoreSight TPDM hardware definitions
coresight-tpdm: Add DSB dataset support
coresight-tpdm: Add integration test support
docs: sysfs: coresight: Add sysfs ABI documentation for TPDM

TPDA - The trace performance monitoring and diagnostics aggregator or
TPDA in short serves as an arbitration and packetization engine for the
performance monitoring and diagnostics network as specified in the QPMDA
(Qualcomm performance monitoring and diagnostics architecture)
specification. The primary use case of the TPDA is to provide
packetization, funneling and timestamping of Monitor data as specified
in the QPMDA specification.
The following patch is to add driver for TPDA.
Coresight: Add TPDA link driver
dt-bindings: arm: Adds CoreSight TPDA hardware definitions

The last patch of this series is a device tree modification, which add
the TPDM and TPDA configuration to device tree for validating.
ARM: dts: msm: Add coresight components for SM8250
ARM: dts: msm: Add tpdm mm/prng for sm8250

Once this series patches are applied properly, the tpdm and tpda nodes
should be observed at the coresight path /sys/bus/coresight/devices
e.g.
/sys/bus/coresight/devices # ls -l | grep tpd
tpda0 -> ../../../devices/platform/soc@0/6004000.tpda/tpda0
tpdm0 -> ../../../devices/platform/soc@0/6c08000.mm.tpdm/tpdm0

We can use the commands are similar to the below to validate TPDMs.
Enable coresight sink first.

echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink
echo 1 > /sys/bus/coresight/devices/tpdm0/enable_source
echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test
echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test
The test data will be collected in the coresight sink which is enabled.
If rwp register of the sink is keeping updating when do
integration_test (by cat tmc_etf0/mgmt/rwp), it means there is data
generated from TPDM to sink.

There must be a tpda between tpdm and the sink. When there are some
other trace event hw components in the same HW block with tpdm, tpdm
and these hw components will connect to the coresight funnel. When
there is only tpdm trace hw in the HW block, tpdm will connect to
tpda directly.

+---------------+ +-------------+
| tpdm@6c08000 | |tpdm@684C000 |
+-------|-------+ +------|------+
| |
+-------|-------+ |
| funnel@6c0b000| |
+-------|-------+ |
| |
+-------|-------+ |
|funnel@6c2d000 | |
+-------|-------+ |
| |
| +---------------+ |
+----- tpda@6004000 -----------+
+-------|-------+
|
+-------|-------+
|funnel@6005000 |
+---------------+

This patch series depends on patch series
"coresight: Add new API to allocate trace source ID values".
https://patchwork.kernel.org/project/linux-arm-kernel/cover/[email protected]/

Changes from V6:
1. Update the commit title and move the changes to right place which
is sorted by address for dtsi changes. -- Konrad Dybcio <[email protected]>

Mao Jinlong (10):
coresight: core: Use IDR for non-cpu bound sources' paths.
Coresight: Add coresight TPDM source driver
dt-bindings: arm: Adds CoreSight TPDM hardware definitions
coresight-tpdm: Add DSB dataset support
coresight-tpdm: Add integration test support
docs: sysfs: coresight: Add sysfs ABI documentation for TPDM
Coresight: Add TPDA link driver
dt-bindings: arm: Adds CoreSight TPDA hardware definitions
arm64: dts: qcom: sm8250: Add coresight components
arm64: dts: qcom: sm8250: Add tpdm mm/prng

.../testing/sysfs-bus-coresight-devices-tpdm | 13 +
.../bindings/arm/coresight-tpda.yaml | 119 ++++
.../bindings/arm/coresight-tpdm.yaml | 99 +++
.../devicetree/bindings/arm/coresight.txt | 7 +
MAINTAINERS | 2 +
arch/arm64/boot/dts/qcom/sm8250.dtsi | 658 ++++++++++++++++++
drivers/hwtracing/coresight/Kconfig | 24 +
drivers/hwtracing/coresight/Makefile | 2 +
drivers/hwtracing/coresight/coresight-core.c | 42 +-
drivers/hwtracing/coresight/coresight-tpda.c | 201 ++++++
drivers/hwtracing/coresight/coresight-tpda.h | 33 +
drivers/hwtracing/coresight/coresight-tpdm.c | 258 +++++++
drivers/hwtracing/coresight/coresight-tpdm.h | 63 ++
include/linux/coresight.h | 1 +
14 files changed, 1510 insertions(+), 12 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpda.yaml
create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c
create mode 100644 drivers/hwtracing/coresight/coresight-tpda.h
create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c
create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h

--
2.17.1



2022-05-09 13:50:52

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 02/10] Coresight: Add coresight TPDM source driver

Add driver to support Coresight device TPDM (Trace, Profiling and
Diagnostics Monitor). TPDM is a monitor to collect data from
different datasets. This change is to add probe/enable/disable
functions for tpdm source.

Signed-off-by: Tao Zhang <[email protected]>
Signed-off-by: Mao Jinlong <[email protected]>
---
drivers/hwtracing/coresight/Kconfig | 13 ++
drivers/hwtracing/coresight/Makefile | 1 +
drivers/hwtracing/coresight/coresight-core.c | 5 +-
drivers/hwtracing/coresight/coresight-tpdm.c | 146 +++++++++++++++++++
drivers/hwtracing/coresight/coresight-tpdm.h | 26 ++++
include/linux/coresight.h | 1 +
6 files changed, 191 insertions(+), 1 deletion(-)
create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c
create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h

diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index 514a9b8086e3..5c506a1cd08f 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -201,4 +201,17 @@ config CORESIGHT_TRBE

To compile this driver as a module, choose M here: the module will be
called coresight-trbe.
+
+config CORESIGHT_TPDM
+ tristate "CoreSight Trace, Profiling & Diagnostics Monitor driver"
+ select CORESIGHT_LINKS_AND_SINKS
+ help
+ This driver provides support for configuring monitor. Monitors are
+ primarily responsible for data set collection and support the
+ ability to collect any permutation of data set types. Monitors are
+ also responsible for interaction with system cross triggering.
+
+ To compile this driver as a module, choose M here: the module will be
+ called coresight-tpdm.
+
endif
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index 329a0c704b87..6bb9b1746bc7 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -25,5 +25,6 @@ obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o
obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o
+obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o
coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \
coresight-cti-sysfs.o
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 23ab16dd9b5d..75fe1781df20 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1047,7 +1047,8 @@ static int coresight_validate_source(struct coresight_device *csdev,
}

if (subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_PROC &&
- subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE) {
+ subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE &&
+ subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY) {
dev_err(&csdev->dev, "wrong device subtype in %s\n", function);
return -EINVAL;
}
@@ -1116,6 +1117,7 @@ int coresight_enable(struct coresight_device *csdev)
per_cpu(tracer_path, cpu) = path;
break;
case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
+ case CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY:
/*
* Use the hash of source's device name as ID
* and map the ID to the pointer of the path.
@@ -1165,6 +1167,7 @@ void coresight_disable(struct coresight_device *csdev)
per_cpu(tracer_path, cpu) = NULL;
break;
case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
+ case CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY:
hash = hashlen_hash(hashlen_string(NULL, dev_name(&csdev->dev)));
/* Find the path by the hash. */
path = idr_find(&path_idr, hash);
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
new file mode 100644
index 000000000000..6a4e2a35053d
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/amba/bus.h>
+#include <linux/bitmap.h>
+#include <linux/coresight.h>
+#include <linux/coresight-pmu.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/fs.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#include "coresight-priv.h"
+#include "coresight-tpdm.h"
+
+DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
+
+/* TPDM enable operations */
+static int tpdm_enable(struct coresight_device *csdev,
+ struct perf_event *event, u32 mode)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ mutex_lock(&drvdata->lock);
+ if (drvdata->enable) {
+ mutex_unlock(&drvdata->lock);
+ return -EBUSY;
+ }
+
+ drvdata->enable = true;
+ mutex_unlock(&drvdata->lock);
+
+ dev_info(drvdata->dev, "TPDM tracing enabled\n");
+ return 0;
+}
+
+/* TPDM disable operations */
+static void tpdm_disable(struct coresight_device *csdev,
+ struct perf_event *event)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ mutex_lock(&drvdata->lock);
+ if (!drvdata->enable) {
+ mutex_unlock(&drvdata->lock);
+ return;
+ }
+
+ drvdata->enable = false;
+ mutex_unlock(&drvdata->lock);
+
+ dev_info(drvdata->dev, "TPDM tracing disabled\n");
+}
+
+static const struct coresight_ops_source tpdm_source_ops = {
+ .enable = tpdm_enable,
+ .disable = tpdm_disable,
+};
+
+static const struct coresight_ops tpdm_cs_ops = {
+ .source_ops = &tpdm_source_ops,
+};
+
+static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
+{
+ struct device *dev = &adev->dev;
+ struct coresight_platform_data *pdata;
+ struct tpdm_drvdata *drvdata;
+ struct coresight_desc desc = { 0 };
+
+ pdata = coresight_get_platform_data(dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ adev->dev.platform_data = pdata;
+
+ /* driver data*/
+ drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+ drvdata->dev = &adev->dev;
+ dev_set_drvdata(dev, drvdata);
+
+ drvdata->base = devm_ioremap_resource(dev, &adev->res);
+ if (!drvdata->base)
+ return -ENOMEM;
+
+ mutex_init(&drvdata->lock);
+
+ /* Set up coresight component description */
+ desc.name = coresight_alloc_device_name(&tpdm_devs, dev);
+ if (!desc.name)
+ return -ENOMEM;
+ desc.type = CORESIGHT_DEV_TYPE_SOURCE;
+ desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY;
+ desc.ops = &tpdm_cs_ops;
+ desc.pdata = adev->dev.platform_data;
+ desc.dev = &adev->dev;
+ drvdata->csdev = coresight_register(&desc);
+ if (IS_ERR(drvdata->csdev))
+ return PTR_ERR(drvdata->csdev);
+
+ /* Decrease pm refcount when probe is done.*/
+ pm_runtime_put(&adev->dev);
+
+ return 0;
+}
+
+static void __exit tpdm_remove(struct amba_device *adev)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
+
+ coresight_unregister(drvdata->csdev);
+}
+
+/*
+ * Different TPDM has different periph id.
+ * The difference is 0-7 bits' value. So ignore 0-7 bits.
+ */
+static struct amba_id tpdm_ids[] = {
+ {
+ .id = 0x000f0e00,
+ .mask = 0x000fff00,
+ },
+ { 0, 0},
+};
+
+static struct amba_driver tpdm_driver = {
+ .drv = {
+ .name = "coresight-tpdm",
+ .owner = THIS_MODULE,
+ .suppress_bind_attrs = true,
+ },
+ .probe = tpdm_probe,
+ .id_table = tpdm_ids,
+ .remove = tpdm_remove,
+};
+
+module_amba_driver(tpdm_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Monitor driver");
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
new file mode 100644
index 000000000000..94a7748a5426
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-tpdm.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _CORESIGHT_CORESIGHT_TPDM_H
+#define _CORESIGHT_CORESIGHT_TPDM_H
+
+/**
+ * struct tpdm_drvdata - specifics associated to an TPDM component
+ * @base: memory mapped base address for this component.
+ * @dev: The device entity associated to this component.
+ * @csdev: component vitals needed by the framework.
+ * @lock: lock for the enable value.
+ * @enable: enable status of the component.
+ */
+
+struct tpdm_drvdata {
+ void __iomem *base;
+ struct device *dev;
+ struct coresight_device *csdev;
+ struct mutex lock;
+ bool enable;
+};
+
+#endif /* _CORESIGHT_CORESIGHT_TPDM_H */
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 247147c11231..a9efac55029d 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -61,6 +61,7 @@ enum coresight_dev_subtype_source {
CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
+ CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY,
};

enum coresight_dev_subtype_helper {
--
2.17.1


2022-05-09 13:54:29

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 03/10] dt-bindings: arm: Adds CoreSight TPDM hardware definitions

Adds new coresight-tpdm.yaml file describing the bindings required
to define tpdm in the device trees.

Reviewed-by: Mike Leach <[email protected]>
Signed-off-by: Tao Zhang <[email protected]>
Signed-off-by: Mao Jinlong <[email protected]>
---
.../bindings/arm/coresight-tpdm.yaml | 99 +++++++++++++++++++
.../devicetree/bindings/arm/coresight.txt | 7 ++
MAINTAINERS | 1 +
3 files changed, 107 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpdm.yaml

diff --git a/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml b/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
new file mode 100644
index 000000000000..451342d3d8b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+# Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/coresight-tpdm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Trace, Profiling and Diagnostics Monitor - TPDM
+
+description: |
+ The TPDM or Monitor serves as data collection component for various dataset
+ types specified in the QPMDA spec. It covers Implementation defined ((ImplDef),
+ Basic Counts (BC), Tenure Counts (TC), Continuous Multi-Bit (CMB), and Discrete
+ Single Bit (DSB). It performs data collection in the data producing clock
+ domain and transfers it to the data collection time domain, generally ATB
+ clock domain.
+
+ The primary use case of the TPDM is to collect data from different data
+ sources and send it to a TPDA for packetization, timestamping, and funneling.
+
+maintainers:
+ - Mao Jinlong <[email protected]>
+ - Tao Zhang <[email protected]>
+
+properties:
+ $nodename:
+ pattern: "^tpdm(@[0-9a-f]+)$"
+ compatible:
+ items:
+ - const: qcom,coresight-tpdm
+ - const: arm,primecell
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: apb_pclk
+
+ out-ports:
+ description: |
+ Output connections from the TPDM to coresight funnle/tpda.
+ $ref: /schemas/graph.yaml#/properties/ports
+ properties:
+ port:
+ description: Output connection from the TPDM to coresight
+ funnel/tpda.
+ $ref: /schemas/graph.yaml#/properties/port
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ # minimum TPDM definition. TPDM connect to coresight funnel.
+ - |
+ tpdm@6980000 {
+ compatible = "qcom,coresight-tpdm", "arm,primecell";
+ reg = <0x6980000 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ tpdm_turing_out_funnel_turing: endpoint {
+ remote-endpoint =
+ <&funnel_turing_in_tpdm_turing>;
+ };
+ };
+ };
+ };
+ # minimum TPDM definition. TPDM connect to coresight TPDA.
+ - |
+ tpdm@684c000 {
+ compatible = "qcom,coresight-tpdm", "arm,primecell";
+ reg = <0x684c000 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ tpdm_prng_out_tpda_qdss: endpoint {
+ remote-endpoint =
+ <&tpda_qdss_in_tpdm_prng>;
+ };
+ };
+ };
+ };
+
+...
diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index c68d93a35b6c..f7ce8af48574 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -52,6 +52,10 @@ its hardware characteristcs.
"arm,coresight-cti", "arm,primecell";
See coresight-cti.yaml for full CTI definitions.

+ - Trace, Profiling and Diagnostics Monitor (TPDM):
+ "qcom,coresight-tpdm", "arm,primecell";
+ See coresight-tpdm.yaml for full TPDM definitions.
+
* reg: physical base address and length of the register
set(s) of the component.

@@ -82,6 +86,9 @@ its hardware characteristcs.
* Required properties for Coresight Cross Trigger Interface (CTI)
See coresight-cti.yaml for full CTI definitions.

+* Required properties for Trace, Profiling and Diagnostics Monitor (TPDM)
+ See coresight-tpdm.yaml for full TPDM definitions.
+
* Required properties for devices that don't show up on the AMBA bus, such as
non-configurable replicators and non-configurable funnels:

diff --git a/MAINTAINERS b/MAINTAINERS
index edc96cdb85e8..28d32b3f3f5c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1978,6 +1978,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git
F: Documentation/ABI/testing/sysfs-bus-coresight-devices-*
F: Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
F: Documentation/devicetree/bindings/arm/coresight-cti.yaml
+F: Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
F: Documentation/devicetree/bindings/arm/coresight.txt
F: Documentation/devicetree/bindings/arm/ete.yaml
F: Documentation/devicetree/bindings/arm/trbe.yaml
--
2.17.1


2022-05-09 13:54:29

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 04/10] coresight-tpdm: Add DSB dataset support

TPDM serves as data collection component for various dataset types.
DSB(Discrete Single Bit) is one of the dataset types. DSB subunit
can be enabled for data collection by writing 1 to the first bit of
DSB_CR register. This change is to add enable/disable function for
DSB dataset by writing DSB_CR register.

Signed-off-by: Tao Zhang <[email protected]>
Signed-off-by: Mao Jinlong <[email protected]>
---
drivers/hwtracing/coresight/coresight-tpdm.c | 58 ++++++++++++++++++++
drivers/hwtracing/coresight/coresight-tpdm.h | 23 ++++++++
2 files changed, 81 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 6a4e2a35053d..70df888ac565 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -20,7 +20,28 @@

DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");

+static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
+{
+ u32 val;
+
+ /* Set the enable bit of DSB control register to 1 */
+ val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
+ val |= TPDM_DSB_CR_ENA;
+ writel_relaxed(val, drvdata->base + TPDM_DSB_CR);
+}
+
/* TPDM enable operations */
+static void _tpdm_enable(struct tpdm_drvdata *drvdata)
+{
+ CS_UNLOCK(drvdata->base);
+
+ /* Check if DSB datasets is present for TPDM. */
+ if (drvdata->datasets & BIT(TPDM_DS_DSB))
+ tpdm_enable_dsb(drvdata);
+
+ CS_LOCK(drvdata->base);
+}
+
static int tpdm_enable(struct coresight_device *csdev,
struct perf_event *event, u32 mode)
{
@@ -32,6 +53,7 @@ static int tpdm_enable(struct coresight_device *csdev,
return -EBUSY;
}

+ _tpdm_enable(drvdata);
drvdata->enable = true;
mutex_unlock(&drvdata->lock);

@@ -39,7 +61,29 @@ static int tpdm_enable(struct coresight_device *csdev,
return 0;
}

+static void tpdm_disable_dsb(struct tpdm_drvdata *drvdata)
+{
+ u32 val;
+
+ /* Set the enable bit of DSB control register to 0 */
+ val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
+ val &= ~TPDM_DSB_CR_ENA;
+ writel_relaxed(val, drvdata->base + TPDM_DSB_CR);
+}
+
/* TPDM disable operations */
+static void _tpdm_disable(struct tpdm_drvdata *drvdata)
+{
+ CS_UNLOCK(drvdata->base);
+
+ /* Check if DSB datasets is present for TPDM. */
+ if (drvdata->datasets & BIT(TPDM_DS_DSB))
+ tpdm_disable_dsb(drvdata);
+
+ CS_LOCK(drvdata->base);
+
+}
+
static void tpdm_disable(struct coresight_device *csdev,
struct perf_event *event)
{
@@ -51,6 +95,7 @@ static void tpdm_disable(struct coresight_device *csdev,
return;
}

+ _tpdm_disable(drvdata);
drvdata->enable = false;
mutex_unlock(&drvdata->lock);

@@ -66,6 +111,18 @@ static const struct coresight_ops tpdm_cs_ops = {
.source_ops = &tpdm_source_ops,
};

+static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
+{
+ int i;
+ u32 pidr;
+
+ CS_UNLOCK(drvdata->base);
+ /* Get the datasets present on the TPDM. */
+ pidr = readl_relaxed(drvdata->base + CORESIGHT_PERIPHIDR0);
+ drvdata->datasets |= pidr & GENMASK(TPDM_DATASETS - 1, 0);
+ CS_LOCK(drvdata->base);
+}
+
static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
{
struct device *dev = &adev->dev;
@@ -104,6 +161,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
if (IS_ERR(drvdata->csdev))
return PTR_ERR(drvdata->csdev);

+ tpdm_init_default_data(drvdata);
/* Decrease pm refcount when probe is done.*/
pm_runtime_put(&adev->dev);

diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index 94a7748a5426..f95aaad9c653 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.h
+++ b/drivers/hwtracing/coresight/coresight-tpdm.h
@@ -6,6 +6,27 @@
#ifndef _CORESIGHT_CORESIGHT_TPDM_H
#define _CORESIGHT_CORESIGHT_TPDM_H

+/* The max number of the datasets that TPDM supports */
+#define TPDM_DATASETS 7
+
+/* DSB Subunit Registers */
+#define TPDM_DSB_CR (0x780)
+/* Enable bit for DSB subunit */
+#define TPDM_DSB_CR_ENA BIT(0)
+
+/**
+ * This enum is for PERIPHIDR0 register of TPDM.
+ * The fields [6:0] of PERIPHIDR0 are used to determine what
+ * interfaces and subunits are present on a given TPDM.
+ *
+ * PERIPHIDR0[0] : Fix to 1 if ImplDef subunit present, else 0
+ * PERIPHIDR0[1] : Fix to 1 if DSB subunit present, else 0
+ */
+enum tpdm_dataset {
+ TPDM_DS_IMPLDEF,
+ TPDM_DS_DSB,
+};
+
/**
* struct tpdm_drvdata - specifics associated to an TPDM component
* @base: memory mapped base address for this component.
@@ -13,6 +34,7 @@
* @csdev: component vitals needed by the framework.
* @lock: lock for the enable value.
* @enable: enable status of the component.
+ * @datasets: The datasets types present of the TPDM.
*/

struct tpdm_drvdata {
@@ -21,6 +43,7 @@ struct tpdm_drvdata {
struct coresight_device *csdev;
struct mutex lock;
bool enable;
+ unsigned long datasets;
};

#endif /* _CORESIGHT_CORESIGHT_TPDM_H */
--
2.17.1


2022-05-09 13:54:33

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 05/10] coresight-tpdm: Add integration test support

Integration test for tpdm can help to generate the data for
verification of the topology during TPDM software bring up.

Sample:
echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink
echo 1 > /sys/bus/coresight/devices/tpdm0/enable_source
echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test
echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test
cat /dev/tmc_etf0 > /data/etf-tpdm0.bin

Signed-off-by: Tao Zhang <[email protected]>
Signed-off-by: Mao Jinlong <[email protected]>
---
drivers/hwtracing/coresight/coresight-tpdm.c | 54 ++++++++++++++++++++
drivers/hwtracing/coresight/coresight-tpdm.h | 14 +++++
2 files changed, 68 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 70df888ac565..57e38aa7d2bd 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -123,6 +123,59 @@ static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
CS_LOCK(drvdata->base);
}

+/*
+ * value 1: 64 bits test data
+ * value 2: 32 bits test data
+ */
+static ssize_t integration_test_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t size)
+{
+ int i, ret = 0;
+ unsigned long val;
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ ret = kstrtoul(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val != 1 && val != 2)
+ return -EINVAL;
+
+ if (!drvdata->enable)
+ return -EINVAL;
+
+ if (val == 1)
+ val = ATBCNTRL_VAL_64;
+ else
+ val = ATBCNTRL_VAL_32;
+ CS_UNLOCK(drvdata->base);
+ writel_relaxed(0x1, drvdata->base + TPDM_ITCNTRL);
+
+ for (i = 1; i < INTEGRATION_TEST_CYCLE; i++)
+ writel_relaxed(val, drvdata->base + TPDM_ITATBCNTRL);
+
+ writel_relaxed(0, drvdata->base + TPDM_ITCNTRL);
+ CS_LOCK(drvdata->base);
+ return size;
+}
+static DEVICE_ATTR_WO(integration_test);
+
+static struct attribute *tpdm_attrs[] = {
+ &dev_attr_integration_test.attr,
+ NULL,
+};
+
+static struct attribute_group tpdm_attr_grp = {
+ .attrs = tpdm_attrs,
+};
+
+static const struct attribute_group *tpdm_attr_grps[] = {
+ &tpdm_attr_grp,
+ NULL,
+};
+
static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
{
struct device *dev = &adev->dev;
@@ -157,6 +210,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
desc.ops = &tpdm_cs_ops;
desc.pdata = adev->dev.platform_data;
desc.dev = &adev->dev;
+ desc.groups = tpdm_attr_grps;
drvdata->csdev = coresight_register(&desc);
if (IS_ERR(drvdata->csdev))
return PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index f95aaad9c653..4aa880794383 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.h
+++ b/drivers/hwtracing/coresight/coresight-tpdm.h
@@ -14,6 +14,20 @@
/* Enable bit for DSB subunit */
#define TPDM_DSB_CR_ENA BIT(0)

+/* TPDM integration test registers */
+#define TPDM_ITATBCNTRL (0xEF0)
+#define TPDM_ITCNTRL (0xF00)
+
+/* Register value for integration test */
+#define ATBCNTRL_VAL_32 0xC00F1409
+#define ATBCNTRL_VAL_64 0xC01F1409
+
+/*
+ * Number of cycles to write value when
+ * integration test.
+ */
+#define INTEGRATION_TEST_CYCLE 10
+
/**
* This enum is for PERIPHIDR0 register of TPDM.
* The fields [6:0] of PERIPHIDR0 are used to determine what
--
2.17.1


2022-05-09 13:54:35

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 06/10] docs: sysfs: coresight: Add sysfs ABI documentation for TPDM

Add API usage document for sysfs API in TPDM driver.

Reviewed-by: Mike Leach <[email protected]>
Signed-off-by: Mao Jinlong <[email protected]>
---
.../ABI/testing/sysfs-bus-coresight-devices-tpdm | 13 +++++++++++++
1 file changed, 13 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
new file mode 100644
index 000000000000..ae0eb128af87
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -0,0 +1,13 @@
+What: /sys/bus/coresight/devices/<tpdm-name>/integration_test
+Date: May 2022
+KernelVersion 5.18
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Run integration test for tpdm. Integration test
+ will generate test data for tpdm. It can help to make
+ sure that the trace path is enabled and the link configurations
+ are fine.
+
+ value to this sysfs node:
+ 1 : Generate 64 bits data
+ 2 : Generate 32 bits data
--
2.17.1


2022-05-09 13:54:42

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 08/10] dt-bindings: arm: Adds CoreSight TPDA hardware definitions

Adds new coresight-tpda.yaml file describing the bindings required
to define tpda in the device trees.

Reviewed-by: Mike Leach <[email protected]>
Signed-off-by: Tao Zhang <[email protected]>
Signed-off-by: Mao Jinlong <[email protected]>
---
.../bindings/arm/coresight-tpda.yaml | 119 ++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 120 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpda.yaml

diff --git a/Documentation/devicetree/bindings/arm/coresight-tpda.yaml b/Documentation/devicetree/bindings/arm/coresight-tpda.yaml
new file mode 100644
index 000000000000..4948ac13e7f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/coresight-tpda.yaml
@@ -0,0 +1,119 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+# Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/coresight-tpda.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Trace, Profiling and Diagnostics Aggregator - TPDA
+
+description: |
+ TPDAs are responsible for packetization and timestamping of data sets
+ utilizing the MIPI STPv2 packet protocol. Pulling data sets from one or
+ more attached TPDM and pushing the resultant (packetized) data out a
+ master ATB interface. Performing an arbitrated ATB interleaving (funneling)
+ task for free-flowing data from TPDM (i.e. CMB and DSB data set flows).
+
+maintainers:
+ - Mao Jinlong <[email protected]>
+ - Tao Zhang <[email protected]>
+
+properties:
+ $nodename:
+ pattern: "^tpda(@[0-9a-f]+)$"
+ compatible:
+ items:
+ - const: qcom,coresight-tpda
+ - const: arm,primecell
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: apb_pclk
+
+ in-ports:
+ type: object
+ description: |
+ Input connections from TPDM to TPDA
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ patternProperties:
+ "^port@[0-9a-f]+$":
+ type: object
+ required:
+ - reg
+
+ required:
+ - '#size-cells'
+ - '#address-cells'
+
+ out-ports:
+ type: object
+ description: |
+ Output connections from the TPDA to legacy CoreSight trace bus.
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port:
+ description:
+ Output connection from the TPDA to legacy CoreSight Trace bus.
+ $ref: /schemas/graph.yaml#/properties/port
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - in-ports
+ - out-ports
+
+additionalProperties: false
+
+examples:
+ # minimum tpda definition.
+ - |
+ tpda@6004000 {
+ compatible = "qcom,coresight-tpda", "arm,primecell";
+ reg = <0x6004000 0x1000>;
+
+ qcom,tpda-atid = <65>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ tpda_qdss_0_in_tpdm_dcc: endpoint {
+ remote-endpoint =
+ <&tpdm_dcc_out_tpda_qdss_0>;
+ };
+ };
+ };
+
+ out-ports {
+ port {
+ tpda_qdss_out_funnel_in0: endpoint {
+ remote-endpoint =
+ <&funnel_in0_in_tpda_qdss>;
+ };
+ };
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 28d32b3f3f5c..5d2d8c0ee340 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1978,6 +1978,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git
F: Documentation/ABI/testing/sysfs-bus-coresight-devices-*
F: Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
F: Documentation/devicetree/bindings/arm/coresight-cti.yaml
+F: Documentation/devicetree/bindings/arm/coresight-tpda.yaml
F: Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
F: Documentation/devicetree/bindings/arm/coresight.txt
F: Documentation/devicetree/bindings/arm/ete.yaml
--
2.17.1


2022-05-09 13:54:48

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 09/10] arm64: dts: qcom: sm8250: Add coresight components

Add coresight components for sm8250. STM/ETM are added.

Signed-off-by: Tao Zhang <[email protected]>
Signed-off-by: Mao Jinlong <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 488 +++++++++++++++++++++++++++
1 file changed, 488 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index af8f22636436..bc86a0e3faa3 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -2708,6 +2708,494 @@
};
};

+ stm@6002000 {
+ compatible = "arm,coresight-stm", "arm,primecell";
+ reg = <0 0x06002000 0 0x1000>, <0 0x16280000 0 0x180000>;
+ reg-names = "stm-base", "stm-stimulus-base";
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ stm_out: endpoint {
+ remote-endpoint = <&funnel0_in7>;
+ };
+ };
+ };
+ };
+
+ funnel@6041000 {
+ compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
+ reg = <0 0x06041000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ funnel0_out: endpoint {
+ remote-endpoint = <&merge_funnel_in0>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@7 {
+ reg = <7>;
+ funnel0_in7: endpoint {
+ remote-endpoint = <&stm_out>;
+ };
+ };
+ };
+ };
+
+ funnel@6042000 {
+ compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
+ reg = <0 0x06042000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ funnel2_out: endpoint {
+ remote-endpoint = <&merge_funnel_in2>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@2 {
+ reg = <4>;
+ funnel2_in5: endpoint {
+ remote-endpoint = <&apss_merge_funnel_out>;
+ };
+ };
+ };
+ };
+
+ funnel@6045000 {
+ compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
+ reg = <0 0x06045000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ funnel_merg_out_funnel_swao: endpoint {
+ remote-endpoint = <&funnel_swao_in_funnel_merg>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <0>;
+ merge_funnel_in0: endpoint {
+ remote-endpoint = <&funnel0_out>;
+ };
+ };
+
+ port@2 {
+ reg = <1>;
+ merge_funnel_in2: endpoint {
+ remote-endpoint = <&funnel2_out>;
+ };
+ };
+ };
+ };
+
+ replicator@6046000 {
+ compatible = "arm,coresight-dynamic-replicator", "arm,primecell";
+ reg = <0 0x06046000 0 0x1000>;
+
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ replicator_out: endpoint {
+ remote-endpoint = <&etr_in>;
+ };
+ };
+ };
+
+ in-ports {
+ port {
+ replicator_cx_in_swao_out: endpoint {
+ remote-endpoint = <&replicator_swao_out_cx_in>;
+ };
+ };
+ };
+ };
+
+ etr@6048000 {
+ compatible = "arm,coresight-tmc", "arm,primecell";
+ reg = <0 0x06048000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+ arm,scatter-gather;
+
+ in-ports {
+ port {
+ etr_in: endpoint {
+ remote-endpoint = <&replicator_out>;
+ };
+ };
+ };
+ };
+
+ funnel@6b04000 {
+ compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
+ arm,primecell-periphid = <0x000bb908>;
+
+ reg = <0 0x06b04000 0 0x1000>;
+ reg-names = "funnel-base";
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ merge_funnel_out: endpoint {
+ remote-endpoint = <&etf_in>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@7 {
+ reg = <7>;
+ funnel_swao_in_funnel_merg: endpoint {
+ remote-endpoint= <&funnel_merg_out_funnel_swao>;
+ };
+ };
+ };
+
+ };
+
+ etf@6b05000 {
+ compatible = "arm,coresight-tmc", "arm,primecell";
+ reg = <0 0x06b05000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ etf_out: endpoint {
+ remote-endpoint = <&replicator_in>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <0>;
+ etf_in: endpoint {
+ remote-endpoint = <&merge_funnel_out>;
+ };
+ };
+ };
+ };
+
+ replicator@6b06000 {
+ compatible = "arm,coresight-dynamic-replicator", "arm,primecell";
+ reg = <0 0x06b06000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ replicator_swao_out_cx_in: endpoint {
+ remote-endpoint = <&replicator_cx_in_swao_out>;
+ };
+ };
+ };
+
+ in-ports {
+ port {
+ replicator_in: endpoint {
+ remote-endpoint = <&etf_out>;
+ };
+ };
+ };
+ };
+
+ etm@7040000 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ reg = <0 0x07040000 0 0x1000>;
+
+ cpu = <&CPU0>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
+
+ out-ports {
+ port {
+ etm0_out: endpoint {
+ remote-endpoint = <&apss_funnel_in0>;
+ };
+ };
+ };
+ };
+
+ etm@7140000 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ reg = <0 0x07140000 0 0x1000>;
+
+ cpu = <&CPU1>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
+
+ out-ports {
+ port {
+ etm1_out: endpoint {
+ remote-endpoint = <&apss_funnel_in1>;
+ };
+ };
+ };
+ };
+
+ etm@7240000 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ reg = <0 0x07240000 0 0x1000>;
+
+ cpu = <&CPU2>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
+
+ out-ports {
+ port {
+ etm2_out: endpoint {
+ remote-endpoint = <&apss_funnel_in2>;
+ };
+ };
+ };
+ };
+
+ etm@7340000 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ reg = <0 0x07340000 0 0x1000>;
+
+ cpu = <&CPU3>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
+
+ out-ports {
+ port {
+ etm3_out: endpoint {
+ remote-endpoint = <&apss_funnel_in3>;
+ };
+ };
+ };
+ };
+
+ etm@7440000 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ reg = <0 0x07440000 0 0x1000>;
+
+ cpu = <&CPU4>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
+
+ out-ports {
+ port {
+ etm4_out: endpoint {
+ remote-endpoint = <&apss_funnel_in4>;
+ };
+ };
+ };
+ };
+
+ etm@7540000 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ reg = <0 0x07540000 0 0x1000>;
+
+ cpu = <&CPU5>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
+
+ out-ports {
+ port {
+ etm5_out: endpoint {
+ remote-endpoint = <&apss_funnel_in5>;
+ };
+ };
+ };
+ };
+
+ etm@7640000 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ reg = <0 0x07640000 0 0x1000>;
+
+ cpu = <&CPU6>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
+
+ out-ports {
+ port {
+ etm6_out: endpoint {
+ remote-endpoint = <&apss_funnel_in6>;
+ };
+ };
+ };
+ };
+
+ etm@7740000 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ reg = <0 0x07740000 0 0x1000>;
+
+ cpu = <&CPU7>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
+
+ out-ports {
+ port {
+ etm7_out: endpoint {
+ remote-endpoint = <&apss_funnel_in7>;
+ };
+ };
+ };
+ };
+
+ funnel@7800000 {
+ compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
+ reg = <0 0x07800000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ apss_funnel_out: endpoint {
+ remote-endpoint = <&apss_merge_funnel_in>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ apss_funnel_in0: endpoint {
+ remote-endpoint = <&etm0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ apss_funnel_in1: endpoint {
+ remote-endpoint = <&etm1_out>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+ apss_funnel_in2: endpoint {
+ remote-endpoint = <&etm2_out>;
+ };
+ };
+
+ port@3 {
+ reg = <3>;
+ apss_funnel_in3: endpoint {
+ remote-endpoint = <&etm3_out>;
+ };
+ };
+
+ port@4 {
+ reg = <4>;
+ apss_funnel_in4: endpoint {
+ remote-endpoint = <&etm4_out>;
+ };
+ };
+
+ port@5 {
+ reg = <5>;
+ apss_funnel_in5: endpoint {
+ remote-endpoint = <&etm5_out>;
+ };
+ };
+
+ port@6 {
+ reg = <6>;
+ apss_funnel_in6: endpoint {
+ remote-endpoint = <&etm6_out>;
+ };
+ };
+
+ port@7 {
+ reg = <7>;
+ apss_funnel_in7: endpoint {
+ remote-endpoint = <&etm7_out>;
+ };
+ };
+ };
+ };
+
+ funnel@7810000 {
+ compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
+ reg = <0 0x07810000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ apss_merge_funnel_out: endpoint {
+ remote-endpoint = <&funnel2_in5>;
+ };
+ };
+ };
+
+ in-ports {
+ port@1 {
+ reg = <0>;
+ apss_merge_funnel_in: endpoint {
+ remote-endpoint = <&apss_funnel_out>;
+ };
+ };
+ };
+ };
+
cdsp: remoteproc@8300000 {
compatible = "qcom,sm8250-cdsp-pas";
reg = <0 0x08300000 0 0x10000>;
--
2.17.1


2022-05-09 13:54:51

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 07/10] Coresight: Add TPDA link driver

TPDA(Trace, Profiling and Diagnostics Aggregator) is
to provide packetization, funneling and timestamping of
TPDM data. Multiple monitors are connected to different
input ports of TPDA.This change is to add tpda
enable/disable/probe functions for coresight tpda driver.

- - - - - - - - - - - -
| TPDM 0| | TPDM 1 | | TPDM 2|
- - - - - - - - - - - -
| | |
|_ _ _ _ _ _ | _ _ _ _ |
| | |
| | |
------------------
| TPDA |
------------------

Signed-off-by: Tao Zhang <[email protected]>
Signed-off-by: Mao Jinlong <[email protected]>
---
drivers/hwtracing/coresight/Kconfig | 11 +
drivers/hwtracing/coresight/Makefile | 1 +
drivers/hwtracing/coresight/coresight-tpda.c | 201 +++++++++++++++++++
drivers/hwtracing/coresight/coresight-tpda.h | 33 +++
4 files changed, 246 insertions(+)
create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c
create mode 100644 drivers/hwtracing/coresight/coresight-tpda.h

diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index 5c506a1cd08f..447919565326 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -205,6 +205,7 @@ config CORESIGHT_TRBE
config CORESIGHT_TPDM
tristate "CoreSight Trace, Profiling & Diagnostics Monitor driver"
select CORESIGHT_LINKS_AND_SINKS
+ select CORESIGHT_TPDA
help
This driver provides support for configuring monitor. Monitors are
primarily responsible for data set collection and support the
@@ -214,4 +215,14 @@ config CORESIGHT_TPDM
To compile this driver as a module, choose M here: the module will be
called coresight-tpdm.

+config CORESIGHT_TPDA
+ tristate "CoreSight Trace, Profiling & Diagnostics Aggregator driver"
+ help
+ This driver provides support for configuring aggregator. This is
+ primarily useful for pulling the data sets from one or more
+ attached monitors and pushing the resultant data out. Multiple
+ monitors are connected on different input ports of TPDA.
+
+ To compile this driver as a module, choose M here: the module will be
+ called coresight-tpda.
endif
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index 6bb9b1746bc7..1712d82e7260 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -26,5 +26,6 @@ obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o
obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o
obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o
+obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o
coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \
coresight-cti-sysfs.o
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
new file mode 100644
index 000000000000..126286e89679
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -0,0 +1,201 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/amba/bus.h>
+#include <linux/bitmap.h>
+#include <linux/coresight.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/fs.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "coresight-priv.h"
+#include "coresight-tpda.h"
+#include "coresight-trace-id.h"
+
+DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
+
+/* Settings pre enabling port control register */
+static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
+{
+ u32 val;
+
+ val = readl_relaxed(drvdata->base + TPDA_CR);
+ /* Bits 6 ~ 12 is for atid value */
+ val |= (drvdata->atid << 6);
+ writel_relaxed(val, drvdata->base + TPDA_CR);
+}
+
+static void tpda_enable_port(struct tpda_drvdata *drvdata, int port)
+{
+ u32 val;
+
+ val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
+ /* Enable the port */
+ val |= TPDA_Pn_CR_ENA;
+ writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
+}
+
+static void _tpda_enable(struct tpda_drvdata *drvdata, int port)
+{
+ CS_UNLOCK(drvdata->base);
+
+ if (!drvdata->enable)
+ tpda_enable_pre_port(drvdata);
+
+ tpda_enable_port(drvdata, port);
+
+ CS_LOCK(drvdata->base);
+}
+
+static int tpda_enable(struct coresight_device *csdev, int inport, int outport)
+{
+ struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ mutex_lock(&drvdata->lock);
+ _tpda_enable(drvdata, inport);
+ drvdata->enable = true;
+ mutex_unlock(&drvdata->lock);
+
+ dev_info(drvdata->dev, "TPDA inport %d enabled\n", inport);
+ return 0;
+}
+
+static void _tpda_disable(struct tpda_drvdata *drvdata, int port)
+{
+ u32 val;
+
+ CS_UNLOCK(drvdata->base);
+
+ val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
+ val &= ~TPDA_Pn_CR_ENA;
+ writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
+
+ CS_LOCK(drvdata->base);
+}
+
+static void tpda_disable(struct coresight_device *csdev, int inport,
+ int outport)
+{
+ struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ mutex_lock(&drvdata->lock);
+ _tpda_disable(drvdata, inport);
+ drvdata->enable = false;
+ mutex_unlock(&drvdata->lock);
+
+ dev_info(drvdata->dev, "TPDA inport %d disabled\n", inport);
+}
+
+static const struct coresight_ops_link tpda_link_ops = {
+ .enable = tpda_enable,
+ .disable = tpda_disable,
+};
+
+static const struct coresight_ops tpda_cs_ops = {
+ .link_ops = &tpda_link_ops,
+};
+
+static int tpda_init_default_data(struct tpda_drvdata *drvdata)
+{
+ int atid;
+ /*
+ * TPDA must has a unique atid. This atid can uniquely
+ * identify the TPDM trace source connect to the TPDA.
+ */
+ atid = coresight_trace_id_get_system_id(coresight_get_trace_id_map());
+ if (atid < 0)
+ return atid;
+
+ drvdata->atid = atid;
+ return 0;
+}
+
+static int tpda_probe(struct amba_device *adev, const struct amba_id *id)
+{
+ int ret;
+ struct device *dev = &adev->dev;
+ struct coresight_platform_data *pdata;
+ struct tpda_drvdata *drvdata;
+ struct coresight_desc desc = { 0 };
+
+ pdata = coresight_get_platform_data(dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ adev->dev.platform_data = pdata;
+
+ drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ drvdata->dev = &adev->dev;
+ dev_set_drvdata(dev, drvdata);
+
+ drvdata->base = devm_ioremap_resource(dev, &adev->res);
+ if (!drvdata->base)
+ return -ENOMEM;
+
+ mutex_init(&drvdata->lock);
+
+ ret = tpda_init_default_data(drvdata);
+ if (ret)
+ return ret;
+
+ desc.name = coresight_alloc_device_name(&tpda_devs, dev);
+ if (!desc.name)
+ return -ENOMEM;
+ desc.type = CORESIGHT_DEV_TYPE_LINK;
+ desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
+ desc.ops = &tpda_cs_ops;
+ desc.pdata = adev->dev.platform_data;
+ desc.dev = &adev->dev;
+ drvdata->csdev = coresight_register(&desc);
+ if (IS_ERR(drvdata->csdev))
+ return PTR_ERR(drvdata->csdev);
+
+ pm_runtime_put(&adev->dev);
+
+ dev_dbg(drvdata->dev, "TPDA initialized\n");
+ return 0;
+}
+
+static void __exit tpda_remove(struct amba_device *adev)
+{
+ struct tpda_drvdata *drvdata = dev_get_drvdata(&adev->dev);
+
+ coresight_unregister(drvdata->csdev);
+}
+
+/*
+ * Different TPDA has different periph id.
+ * The difference is 0-7 bits' value. So ignore 0-7 bits.
+ */
+static struct amba_id tpda_ids[] = {
+ {
+ .id = 0x000f0f00,
+ .mask = 0x000fff00,
+ },
+ { 0, 0},
+};
+
+static struct amba_driver tpda_driver = {
+ .drv = {
+ .name = "coresight-tpda",
+ .owner = THIS_MODULE,
+ .suppress_bind_attrs = true,
+ },
+ .probe = tpda_probe,
+ .remove = tpda_remove,
+ .id_table = tpda_ids,
+};
+
+module_amba_driver(tpda_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Aggregator driver");
diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h
new file mode 100644
index 000000000000..6df1b72b3b76
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-tpda.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _CORESIGHT_CORESIGHT_TPDA_H
+#define _CORESIGHT_CORESIGHT_TPDA_H
+
+#define TPDA_CR (0x000)
+#define TPDA_Pn_CR(n) (0x004 + (n * 4))
+/* Aggregator port enable bit */
+#define TPDA_Pn_CR_ENA BIT(0)
+
+#define TPDA_MAX_INPORTS 32
+
+/**
+ * struct tpda_drvdata - specifics associated to an TPDA component
+ * @base: memory mapped base address for this component.
+ * @dev: The device entity associated to this component.
+ * @csdev: component vitals needed by the framework.
+ * @lock: lock for the enable value.
+ * @enable: enable status of the component.
+ */
+struct tpda_drvdata {
+ void __iomem *base;
+ struct device *dev;
+ struct coresight_device *csdev;
+ struct mutex lock;
+ bool enable;
+ u32 atid;
+};
+
+#endif /* _CORESIGHT_CORESIGHT_TPDA_H */
--
2.17.1


2022-05-09 13:55:02

by Mao Jinlong

[permalink] [raw]
Subject: [PATCH v7 10/10] arm64: dts: qcom: sm8250: Add tpdm mm/prng

Add tpdm mm and tpdm prng for sm8250.

+---------------+ +-------------+
| tpdm@6c08000 | |tpdm@684C000 |
+-------|-------+ +------|------+
| |
+-------|-------+ |
| funnel@6c0b000| |
+-------|-------+ |
| |
+-------|-------+ |
|funnel@6c2d000 | |
+-------|-------+ |
| |
| +---------------+ |
+----- tpda@6004000 -----------+
+-------|-------+
|
+-------|-------+
|funnel@6005000 |
+---------------+

Signed-off-by: Mao Jinlong <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 170 +++++++++++++++++++++++++++
1 file changed, 170 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index bc86a0e3faa3..b3cc99e5f5dd 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -2725,6 +2725,73 @@
};
};

+ tpda@6004000 {
+ compatible = "arm,primecell";
+ reg = <0 0x06004000 0 0x1000>;
+ reg-names = "tpda-base";
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ reg = <0>;
+ tpda_out_funnel_qatb: endpoint {
+ remote-endpoint = <&funnel_qatb_in_tpda>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@9 {
+ reg = <9>;
+ tpda_9_in_tpdm_mm: endpoint {
+ remote-endpoint = <&tpdm_mm_out_tpda9>;
+ };
+ };
+
+ port@23 {
+ reg = <23>;
+ tpda_23_in_tpdm_prng: endpoint {
+ remote-endpoint = <&tpdm_prng_out_tpda_23>;
+ };
+ };
+ };
+ };
+
+ funnel@6005000 {
+ compatible = "arm,primecell";
+
+ reg = <0 0x06005000 0 0x1000>;
+ reg-names = "funnel-base";
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ funnel_qatb_out_funnel_in0: endpoint {
+ remote-endpoint = <&funnel_in0_in_funnel_qatb>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ funnel_qatb_in_tpda: endpoint {
+ remote-endpoint = <&tpda_out_funnel_qatb>;
+ };
+ };
+ };
+ };
+
funnel@6041000 {
compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
reg = <0 0x06041000 0 0x1000>;
@@ -2744,6 +2811,13 @@
#address-cells = <1>;
#size-cells = <0>;

+ port@6 {
+ reg = <6>;
+ funnel_in0_in_funnel_qatb: endpoint {
+ remote-endpoint = <&funnel_qatb_out_funnel_in0>;
+ };
+ };
+
port@7 {
reg = <7>;
funnel0_in7: endpoint {
@@ -2858,6 +2932,23 @@
};
};

+ tpdm@684c000 {
+ compatible = "arm,primecell";
+ reg = <0 0x0684c000 0 0x1000>;
+ reg-names = "tpdm-base";
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ tpdm_prng_out_tpda_23: endpoint {
+ remote-endpoint = <&tpda_23_in_tpdm_prng>;
+ };
+ };
+ };
+ };
+
funnel@6b04000 {
compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
arm,primecell-periphid = <0x000bb908>;
@@ -2942,6 +3033,85 @@
};
};

+ tpdm@6c08000 {
+ compatible = "arm,primecell";
+ reg = <0 0x06c08000 0 0x1000>;
+ reg-names = "tpdm-base";
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ tpdm_mm_out_funnel_dl_mm: endpoint {
+ remote-endpoint = <&funnel_dl_mm_in_tpdm_mm>;
+ };
+ };
+ };
+ };
+
+ funnel@6c0b000 {
+ compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
+
+ reg = <0 0x06c0b000 0 0x1000>;
+ reg-names = "funnel-base";
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ funnel_dl_mm_out_funnel_dl_center: endpoint {
+ remote-endpoint = <&funnel_dl_center_in_funnel_dl_mm>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@3 {
+ reg = <3>;
+ funnel_dl_mm_in_tpdm_mm: endpoint {
+ remote-endpoint = <&tpdm_mm_out_funnel_dl_mm>;
+ };
+ };
+ };
+ };
+
+ funnel@6c2d000 {
+ compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
+
+ reg = <0 0x06c2d000 0 0x1000>;
+ reg-names = "funnel-base";
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port {
+ tpdm_mm_out_tpda9: endpoint {
+ remote-endpoint = <&tpda_9_in_tpdm_mm>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@2 {
+ reg = <2>;
+ funnel_dl_center_in_funnel_dl_mm: endpoint {
+ remote-endpoint = <&funnel_dl_mm_out_funnel_dl_center>;
+ };
+ };
+ };
+ };
+
etm@7040000 {
compatible = "arm,coresight-etm4x", "arm,primecell";
reg = <0 0x07040000 0 0x1000>;
--
2.17.1


2022-05-23 08:57:35

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v7 02/10] Coresight: Add coresight TPDM source driver

Hi

On 09/05/2022 14:39, Mao Jinlong wrote:
> Add driver to support Coresight device TPDM (Trace, Profiling and
> Diagnostics Monitor). TPDM is a monitor to collect data from
> different datasets. This change is to add probe/enable/disable
> functions for tpdm source.
>
> Signed-off-by: Tao Zhang <[email protected]>
> Signed-off-by: Mao Jinlong <[email protected]>
> ---
> drivers/hwtracing/coresight/Kconfig | 13 ++
> drivers/hwtracing/coresight/Makefile | 1 +
> drivers/hwtracing/coresight/coresight-core.c | 5 +-
> drivers/hwtracing/coresight/coresight-tpdm.c | 146 +++++++++++++++++++
> drivers/hwtracing/coresight/coresight-tpdm.h | 26 ++++
> include/linux/coresight.h | 1 +
> 6 files changed, 191 insertions(+), 1 deletion(-)
> create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c
> create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h
>
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 514a9b8086e3..5c506a1cd08f 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -201,4 +201,17 @@ config CORESIGHT_TRBE
>
> To compile this driver as a module, choose M here: the module will be
> called coresight-trbe.
> +
> +config CORESIGHT_TPDM
> + tristate "CoreSight Trace, Profiling & Diagnostics Monitor driver"
> + select CORESIGHT_LINKS_AND_SINKS
> + help
> + This driver provides support for configuring monitor. Monitors are
> + primarily responsible for data set collection and support the
> + ability to collect any permutation of data set types. Monitors are
> + also responsible for interaction with system cross triggering.

I find the last statement a bit confusing. Could this be :

"Monitors are also connected to the cross triggers."

> +
> + To compile this driver as a module, choose M here: the module will be
> + called coresight-tpdm.
> +
> endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 329a0c704b87..6bb9b1746bc7 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -25,5 +25,6 @@ obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
> obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o
> obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
> obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o
> +obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o
> coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \
> coresight-cti-sysfs.o
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 23ab16dd9b5d..75fe1781df20 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1047,7 +1047,8 @@ static int coresight_validate_source(struct coresight_device *csdev,
> }
>
> if (subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_PROC &&
> - subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE) {
> + subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE &&
> + subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY) {
> dev_err(&csdev->dev, "wrong device subtype in %s\n", function);
> return -EINVAL;
> }
> @@ -1116,6 +1117,7 @@ int coresight_enable(struct coresight_device *csdev)
> per_cpu(tracer_path, cpu) = path;
> break;
> case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
> + case CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY:
> /*
> * Use the hash of source's device name as ID
> * and map the ID to the pointer of the path.
> @@ -1165,6 +1167,7 @@ void coresight_disable(struct coresight_device *csdev)
> per_cpu(tracer_path, cpu) = NULL;
> break;
> case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
> + case CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY:
> hash = hashlen_hash(hashlen_string(NULL, dev_name(&csdev->dev)));
> /* Find the path by the hash. */
> path = idr_find(&path_idr, hash);
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> new file mode 100644
> index 000000000000..6a4e2a35053d
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/amba/bus.h>
> +#include <linux/bitmap.h>
> +#include <linux/coresight.h>
> +#include <linux/coresight-pmu.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include "coresight-priv.h"
> +#include "coresight-tpdm.h"
> +
> +DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
> +
> +/* TPDM enable operations */
> +static int tpdm_enable(struct coresight_device *csdev,
> + struct perf_event *event, u32 mode)
> +{
> + struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + mutex_lock(&drvdata->lock);
> + if (drvdata->enable) {
> + mutex_unlock(&drvdata->lock);
> + return -EBUSY;
> + }
> +
> + drvdata->enable = true;
> + mutex_unlock(&drvdata->lock);
> +
> + dev_info(drvdata->dev, "TPDM tracing enabled\n");
> + return 0;
> +}
> +
> +/* TPDM disable operations */
> +static void tpdm_disable(struct coresight_device *csdev,
> + struct perf_event *event)
> +{
> + struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + mutex_lock(&drvdata->lock);
> + if (!drvdata->enable) {
> + mutex_unlock(&drvdata->lock);
> + return;
> + }
> +
> + drvdata->enable = false;
> + mutex_unlock(&drvdata->lock);
> +
> + dev_info(drvdata->dev, "TPDM tracing disabled\n");
> +}
> +
> +static const struct coresight_ops_source tpdm_source_ops = {
> + .enable = tpdm_enable,
> + .disable = tpdm_disable,
> +};
> +
> +static const struct coresight_ops tpdm_cs_ops = {
> + .source_ops = &tpdm_source_ops,
> +};
> +
> +static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> + struct device *dev = &adev->dev;
> + struct coresight_platform_data *pdata;
> + struct tpdm_drvdata *drvdata;
> + struct coresight_desc desc = { 0 };
> +
> + pdata = coresight_get_platform_data(dev);
> + if (IS_ERR(pdata))
> + return PTR_ERR(pdata);
> + adev->dev.platform_data = pdata;
> +
> + /* driver data*/
> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> + drvdata->dev = &adev->dev;
> + dev_set_drvdata(dev, drvdata);
> +
> + drvdata->base = devm_ioremap_resource(dev, &adev->res);
> + if (!drvdata->base)
> + return -ENOMEM;
> +
> + mutex_init(&drvdata->lock);
> +
> + /* Set up coresight component description */
> + desc.name = coresight_alloc_device_name(&tpdm_devs, dev);
> + if (!desc.name)
> + return -ENOMEM;
> + desc.type = CORESIGHT_DEV_TYPE_SOURCE;
> + desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY;
> + desc.ops = &tpdm_cs_ops;
> + desc.pdata = adev->dev.platform_data;
> + desc.dev = &adev->dev;

desc.access must be initialised here.

desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);

> + drvdata->csdev = coresight_register(&desc);
> + if (IS_ERR(drvdata->csdev))
> + return PTR_ERR(drvdata->csdev);
> +
> + /* Decrease pm refcount when probe is done.*/
> + pm_runtime_put(&adev->dev);
> +
> + return 0;
> +}
> +
> +static void __exit tpdm_remove(struct amba_device *adev)
> +{
> + struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> + coresight_unregister(drvdata->csdev);
> +}
> +
> +/*
> + * Different TPDM has different periph id.
> + * The difference is 0-7 bits' value. So ignore 0-7 bits.
> + */
> +static struct amba_id tpdm_ids[] = {
> + {
> + .id = 0x000f0e00,
> + .mask = 0x000fff00,
> + },
> + { 0, 0},
> +};
> +
> +static struct amba_driver tpdm_driver = {
> + .drv = {
> + .name = "coresight-tpdm",
> + .owner = THIS_MODULE,
> + .suppress_bind_attrs = true,
> + },
> + .probe = tpdm_probe,
> + .id_table = tpdm_ids,
> + .remove = tpdm_remove,
> +};
> +
> +module_amba_driver(tpdm_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Monitor driver");
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> new file mode 100644
> index 000000000000..94a7748a5426
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef _CORESIGHT_CORESIGHT_TPDM_H
> +#define _CORESIGHT_CORESIGHT_TPDM_H
> +
> +/**
> + * struct tpdm_drvdata - specifics associated to an TPDM component
> + * @base: memory mapped base address for this component.
> + * @dev: The device entity associated to this component.
> + * @csdev: component vitals needed by the framework.
> + * @lock: lock for the enable value.
> + * @enable: enable status of the component.
> + */
> +
> +struct tpdm_drvdata {
> + void __iomem *base;
> + struct device *dev;
> + struct coresight_device *csdev;
> + struct mutex lock;

Why mutex lock ? Couldn't this be a spinlock ?

> + bool enable;
> +};
> +
> +#endif /* _CORESIGHT_CORESIGHT_TPDM_H */
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 247147c11231..a9efac55029d 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -61,6 +61,7 @@ enum coresight_dev_subtype_source {
> CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
> CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
> CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
> + CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY,

super minor nit: I find the choice of name a bit odd.
We could simply make it something like :

CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS:

Suzuki

> };
>
> enum coresight_dev_subtype_helper {


2022-05-23 09:03:02

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v7 03/10] dt-bindings: arm: Adds CoreSight TPDM hardware definitions

Cc: Rob Herring

Hi

Please Cc Rob for any new DT/Yaml additions. I have done so now.




On 09/05/2022 14:39, Mao Jinlong wrote:
> Adds new coresight-tpdm.yaml file describing the bindings required
> to define tpdm in the device trees.
>
> Reviewed-by: Mike Leach <[email protected]>
> Signed-off-by: Tao Zhang <[email protected]>
> Signed-off-by: Mao Jinlong <[email protected]>

Acked-by: Suzuki K Poulose <[email protected]>

> ---
> .../bindings/arm/coresight-tpdm.yaml | 99 +++++++++++++++++++
> .../devicetree/bindings/arm/coresight.txt | 7 ++
> MAINTAINERS | 1 +
> 3 files changed, 107 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml b/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
> new file mode 100644
> index 000000000000..451342d3d8b7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +# Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/coresight-tpdm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Trace, Profiling and Diagnostics Monitor - TPDM
> +
> +description: |
> + The TPDM or Monitor serves as data collection component for various dataset
> + types specified in the QPMDA spec. It covers Implementation defined ((ImplDef),
> + Basic Counts (BC), Tenure Counts (TC), Continuous Multi-Bit (CMB), and Discrete
> + Single Bit (DSB). It performs data collection in the data producing clock
> + domain and transfers it to the data collection time domain, generally ATB
> + clock domain.
> +
> + The primary use case of the TPDM is to collect data from different data
> + sources and send it to a TPDA for packetization, timestamping, and funneling.
> +
> +maintainers:
> + - Mao Jinlong <[email protected]>
> + - Tao Zhang <[email protected]>
> +
> +properties:
> + $nodename:
> + pattern: "^tpdm(@[0-9a-f]+)$"
> + compatible:
> + items:
> + - const: qcom,coresight-tpdm
> + - const: arm,primecell
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: apb_pclk
> +
> + out-ports:
> + description: |
> + Output connections from the TPDM to coresight funnle/tpda.
> + $ref: /schemas/graph.yaml#/properties/ports
> + properties:
> + port:
> + description: Output connection from the TPDM to coresight
> + funnel/tpda.
> + $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> + # minimum TPDM definition. TPDM connect to coresight funnel.
> + - |
> + tpdm@6980000 {
> + compatible = "qcom,coresight-tpdm", "arm,primecell";
> + reg = <0x6980000 0x1000>;
> +
> + clocks = <&aoss_qmp>;
> + clock-names = "apb_pclk";
> +
> + out-ports {
> + port {
> + tpdm_turing_out_funnel_turing: endpoint {
> + remote-endpoint =
> + <&funnel_turing_in_tpdm_turing>;
> + };
> + };
> + };
> + };
> + # minimum TPDM definition. TPDM connect to coresight TPDA.
> + - |
> + tpdm@684c000 {
> + compatible = "qcom,coresight-tpdm", "arm,primecell";
> + reg = <0x684c000 0x1000>;
> +
> + clocks = <&aoss_qmp>;
> + clock-names = "apb_pclk";
> +
> + out-ports {
> + port {
> + tpdm_prng_out_tpda_qdss: endpoint {
> + remote-endpoint =
> + <&tpda_qdss_in_tpdm_prng>;
> + };
> + };
> + };
> + };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index c68d93a35b6c..f7ce8af48574 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -52,6 +52,10 @@ its hardware characteristcs.
> "arm,coresight-cti", "arm,primecell";
> See coresight-cti.yaml for full CTI definitions.
>
> + - Trace, Profiling and Diagnostics Monitor (TPDM):
> + "qcom,coresight-tpdm", "arm,primecell";
> + See coresight-tpdm.yaml for full TPDM definitions.
> +
> * reg: physical base address and length of the register
> set(s) of the component.
>
> @@ -82,6 +86,9 @@ its hardware characteristcs.
> * Required properties for Coresight Cross Trigger Interface (CTI)
> See coresight-cti.yaml for full CTI definitions.
>
> +* Required properties for Trace, Profiling and Diagnostics Monitor (TPDM)
> + See coresight-tpdm.yaml for full TPDM definitions.
> +
> * Required properties for devices that don't show up on the AMBA bus, such as
> non-configurable replicators and non-configurable funnels:
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index edc96cdb85e8..28d32b3f3f5c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1978,6 +1978,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git
> F: Documentation/ABI/testing/sysfs-bus-coresight-devices-*
> F: Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
> F: Documentation/devicetree/bindings/arm/coresight-cti.yaml
> +F: Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
> F: Documentation/devicetree/bindings/arm/coresight.txt
> F: Documentation/devicetree/bindings/arm/ete.yaml
> F: Documentation/devicetree/bindings/arm/trbe.yaml


2022-05-23 09:12:11

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v7 04/10] coresight-tpdm: Add DSB dataset support

Hi

On 09/05/2022 14:39, Mao Jinlong wrote:
> TPDM serves as data collection component for various dataset types.
> DSB(Discrete Single Bit) is one of the dataset types. DSB subunit
> can be enabled for data collection by writing 1 to the first bit of
> DSB_CR register. This change is to add enable/disable function for
> DSB dataset by writing DSB_CR register.

The patch looks good to me, except for some minor comment below.

>
> Signed-off-by: Tao Zhang <[email protected]>
> Signed-off-by: Mao Jinlong <[email protected]>
> ---
> drivers/hwtracing/coresight/coresight-tpdm.c | 58 ++++++++++++++++++++
> drivers/hwtracing/coresight/coresight-tpdm.h | 23 ++++++++
> 2 files changed, 81 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 6a4e2a35053d..70df888ac565 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -20,7 +20,28 @@
>
> DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>
> +static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
> +{
> + u32 val;
> +
> + /* Set the enable bit of DSB control register to 1 */
> + val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
> + val |= TPDM_DSB_CR_ENA;
> + writel_relaxed(val, drvdata->base + TPDM_DSB_CR);
> +}
> +
> /* TPDM enable operations */
> +static void _tpdm_enable(struct tpdm_drvdata *drvdata)
> +{
> + CS_UNLOCK(drvdata->base);
> +
> + /* Check if DSB datasets is present for TPDM. */
> + if (drvdata->datasets & BIT(TPDM_DS_DSB))
> + tpdm_enable_dsb(drvdata);
> +
> + CS_LOCK(drvdata->base);
> +}
> +
> static int tpdm_enable(struct coresight_device *csdev,
> struct perf_event *event, u32 mode)
> {
> @@ -32,6 +53,7 @@ static int tpdm_enable(struct coresight_device *csdev,
> return -EBUSY;
> }
>
> + _tpdm_enable(drvdata);
> drvdata->enable = true;
> mutex_unlock(&drvdata->lock);
>
> @@ -39,7 +61,29 @@ static int tpdm_enable(struct coresight_device *csdev,
> return 0;
> }
>
> +static void tpdm_disable_dsb(struct tpdm_drvdata *drvdata)
> +{
> + u32 val;
> +
> + /* Set the enable bit of DSB control register to 0 */
> + val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
> + val &= ~TPDM_DSB_CR_ENA;
> + writel_relaxed(val, drvdata->base + TPDM_DSB_CR);
> +}
> +
> /* TPDM disable operations */
> +static void _tpdm_disable(struct tpdm_drvdata *drvdata)
> +{
> + CS_UNLOCK(drvdata->base);
> +
> + /* Check if DSB datasets is present for TPDM. */
> + if (drvdata->datasets & BIT(TPDM_DS_DSB))
> + tpdm_disable_dsb(drvdata);
> +
> + CS_LOCK(drvdata->base);
> +

nit: extra new line.
> +}
> +
> static void tpdm_disable(struct coresight_device *csdev,
> struct perf_event *event)
> {
> @@ -51,6 +95,7 @@ static void tpdm_disable(struct coresight_device *csdev,
> return;
> }
>
> + _tpdm_disable(drvdata);
> drvdata->enable = false;
> mutex_unlock(&drvdata->lock);
>
> @@ -66,6 +111,18 @@ static const struct coresight_ops tpdm_cs_ops = {
> .source_ops = &tpdm_source_ops,
> };
>
> +static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
> +{
> + int i;
> + u32 pidr;
> +
> + CS_UNLOCK(drvdata->base);
> + /* Get the datasets present on the TPDM. */
> + pidr = readl_relaxed(drvdata->base + CORESIGHT_PERIPHIDR0);
> + drvdata->datasets |= pidr & GENMASK(TPDM_DATASETS - 1, 0);
> + CS_LOCK(drvdata->base);
> +}
> +
> static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
> {
> struct device *dev = &adev->dev;
> @@ -104,6 +161,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
> if (IS_ERR(drvdata->csdev))
> return PTR_ERR(drvdata->csdev);
>
> + tpdm_init_default_data(drvdata);
> /* Decrease pm refcount when probe is done.*/
> pm_runtime_put(&adev->dev);
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index 94a7748a5426..f95aaad9c653 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -6,6 +6,27 @@
> #ifndef _CORESIGHT_CORESIGHT_TPDM_H
> #define _CORESIGHT_CORESIGHT_TPDM_H
>
> +/* The max number of the datasets that TPDM supports */
> +#define TPDM_DATASETS 7
> +
> +/* DSB Subunit Registers */
> +#define TPDM_DSB_CR (0x780)
> +/* Enable bit for DSB subunit */
> +#define TPDM_DSB_CR_ENA BIT(0)
> +
> +/**
> + * This enum is for PERIPHIDR0 register of TPDM.
> + * The fields [6:0] of PERIPHIDR0 are used to determine what
> + * interfaces and subunits are present on a given TPDM.
> + *
> + * PERIPHIDR0[0] : Fix to 1 if ImplDef subunit present, else 0
> + * PERIPHIDR0[1] : Fix to 1 if DSB subunit present, else 0
> + */
> +enum tpdm_dataset {
> + TPDM_DS_IMPLDEF,
> + TPDM_DS_DSB,
> +};

Please could we name this explicitly to indicate the register field they
appear in ? e.g,

#define TPDM_PIDR0_DS_IMPDEF BIT(0)
#define TPDM_PIDR0_DS_DSB BIT(1)

Suzuki

2022-05-23 09:17:34

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v7 05/10] coresight-tpdm: Add integration test support

On 09/05/2022 14:39, Mao Jinlong wrote:
> Integration test for tpdm can help to generate the data for
> verification of the topology during TPDM software bring up.
>
> Sample:
> echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink
> echo 1 > /sys/bus/coresight/devices/tpdm0/enable_source
> echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test
> echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test
> cat /dev/tmc_etf0 > /data/etf-tpdm0.bin
>
> Signed-off-by: Tao Zhang <[email protected]>
> Signed-off-by: Mao Jinlong <[email protected]>

Please could we stick this under a sub-Kconfig entry, like
we did for the CTI ?

CONFIG_CORESIGHT_TPMD_INTEGRATION_TEST

> ---
> drivers/hwtracing/coresight/coresight-tpdm.c | 54 ++++++++++++++++++++
> drivers/hwtracing/coresight/coresight-tpdm.h | 14 +++++
> 2 files changed, 68 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 70df888ac565..57e38aa7d2bd 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -123,6 +123,59 @@ static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
> CS_LOCK(drvdata->base);
> }
>
> +/*
> + * value 1: 64 bits test data
> + * value 2: 32 bits test data
> + */
> +static ssize_t integration_test_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t size)
> +{
> + int i, ret = 0;
> + unsigned long val;
> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> + ret = kstrtoul(buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + if (val != 1 && val != 2)
> + return -EINVAL;
> +
> + if (!drvdata->enable)
> + return -EINVAL;
> +
> + if (val == 1)
> + val = ATBCNTRL_VAL_64;
> + else
> + val = ATBCNTRL_VAL_32;
> + CS_UNLOCK(drvdata->base);
> + writel_relaxed(0x1, drvdata->base + TPDM_ITCNTRL);
> +
> + for (i = 1; i < INTEGRATION_TEST_CYCLE; i++)

super minor nit : It is a bit un-natural, not to have i = 0;

Rest looks fine to me .

Suzuki


> + writel_relaxed(val, drvdata->base + TPDM_ITATBCNTRL);
> +
> + writel_relaxed(0, drvdata->base + TPDM_ITCNTRL);
> + CS_LOCK(drvdata->base);
> + return size;
> +}
> +static DEVICE_ATTR_WO(integration_test);
> +
> +static struct attribute *tpdm_attrs[] = {
> + &dev_attr_integration_test.attr,
> + NULL,
> +};
> +
> +static struct attribute_group tpdm_attr_grp = {
> + .attrs = tpdm_attrs,
> +};
> +
> +static const struct attribute_group *tpdm_attr_grps[] = {
> + &tpdm_attr_grp,
> + NULL,
> +};
> +
> static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
> {
> struct device *dev = &adev->dev;
> @@ -157,6 +210,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
> desc.ops = &tpdm_cs_ops;
> desc.pdata = adev->dev.platform_data;
> desc.dev = &adev->dev;
> + desc.groups = tpdm_attr_grps;
> drvdata->csdev = coresight_register(&desc);
> if (IS_ERR(drvdata->csdev))
> return PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index f95aaad9c653..4aa880794383 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -14,6 +14,20 @@
> /* Enable bit for DSB subunit */
> #define TPDM_DSB_CR_ENA BIT(0)
>
> +/* TPDM integration test registers */
> +#define TPDM_ITATBCNTRL (0xEF0)
> +#define TPDM_ITCNTRL (0xF00)
> +
> +/* Register value for integration test */
> +#define ATBCNTRL_VAL_32 0xC00F1409
> +#define ATBCNTRL_VAL_64 0xC01F1409
> +
> +/*
> + * Number of cycles to write value when
> + * integration test.
> + */
> +#define INTEGRATION_TEST_CYCLE 10
> +
> /**
> * This enum is for PERIPHIDR0 register of TPDM.
> * The fields [6:0] of PERIPHIDR0 are used to determine what


2022-05-23 09:25:10

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v7 04/10] coresight-tpdm: Add DSB dataset support

On 23/05/2022 10:11, Suzuki K Poulose wrote:
> Hi
>
> On 09/05/2022 14:39, Mao Jinlong wrote:
>> TPDM serves as data collection component for various dataset types.
>> DSB(Discrete Single Bit) is one of the dataset types. DSB subunit
>> can be enabled for data collection by writing 1 to the first bit of
>> DSB_CR register. This change is to add enable/disable function for
>> DSB dataset by writing DSB_CR register.
>
> The patch looks good to me, except for some minor comment below.
>
>>
>> Signed-off-by: Tao Zhang <[email protected]>
>> Signed-off-by: Mao Jinlong <[email protected]>
>> ---
>>   drivers/hwtracing/coresight/coresight-tpdm.c | 58 ++++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tpdm.h | 23 ++++++++
>>   2 files changed, 81 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c
>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>> index 6a4e2a35053d..70df888ac565 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -20,7 +20,28 @@
>>   DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>> +static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>> +{
>> +    u32 val;
>> +
>> +    /* Set the enable bit of DSB control register to 1 */
>> +    val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
>> +    val |= TPDM_DSB_CR_ENA;
>> +    writel_relaxed(val, drvdata->base + TPDM_DSB_CR);
>> +}
>> +



>>   /* TPDM enable operations */
>> +static void _tpdm_enable(struct tpdm_drvdata *drvdata)


>>   /* TPDM disable operations */
>> +static void _tpdm_disable(struct tpdm_drvdata *drvdata)

Missed this. The general convention is to use:

__tpdm_disable()
__tpdm_enable();

So, please switch to the names above.

Suzuki

2022-05-23 09:49:50

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v7 07/10] Coresight: Add TPDA link driver

On 09/05/2022 14:39, Mao Jinlong wrote:
> TPDA(Trace, Profiling and Diagnostics Aggregator) is
> to provide packetization, funneling and timestamping of
> TPDM data. Multiple monitors are connected to different
> input ports of TPDA.This change is to add tpda
> enable/disable/probe functions for coresight tpda driver.
>
> - - - - - - - - - - - -
> | TPDM 0| | TPDM 1 | | TPDM 2|
> - - - - - - - - - - - -
> | | |
> |_ _ _ _ _ _ | _ _ _ _ |
> | | |
> | | |
> ------------------
> | TPDA |
> ------------------
>
> Signed-off-by: Tao Zhang <[email protected]>
> Signed-off-by: Mao Jinlong <[email protected]>
> ---
> drivers/hwtracing/coresight/Kconfig | 11 +
> drivers/hwtracing/coresight/Makefile | 1 +
> drivers/hwtracing/coresight/coresight-tpda.c | 201 +++++++++++++++++++
> drivers/hwtracing/coresight/coresight-tpda.h | 33 +++
> 4 files changed, 246 insertions(+)
> create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c
> create mode 100644 drivers/hwtracing/coresight/coresight-tpda.h
>
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 5c506a1cd08f..447919565326 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -205,6 +205,7 @@ config CORESIGHT_TRBE
> config CORESIGHT_TPDM
> tristate "CoreSight Trace, Profiling & Diagnostics Monitor driver"
> select CORESIGHT_LINKS_AND_SINKS
> + select CORESIGHT_TPDA
> help
> This driver provides support for configuring monitor. Monitors are
> primarily responsible for data set collection and support the
> @@ -214,4 +215,14 @@ config CORESIGHT_TPDM
> To compile this driver as a module, choose M here: the module will be
> called coresight-tpdm.
>
> +config CORESIGHT_TPDA
> + tristate "CoreSight Trace, Profiling & Diagnostics Aggregator driver"
> + help
> + This driver provides support for configuring aggregator. This is
> + primarily useful for pulling the data sets from one or more
> + attached monitors and pushing the resultant data out. Multiple
> + monitors are connected on different input ports of TPDA.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called coresight-tpda.
> endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 6bb9b1746bc7..1712d82e7260 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -26,5 +26,6 @@ obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o
> obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
> obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o
> obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o
> +obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o
> coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \
> coresight-cti-sysfs.o
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> new file mode 100644
> index 000000000000..126286e89679
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/amba/bus.h>
> +#include <linux/bitmap.h>
> +#include <linux/coresight.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include "coresight-priv.h"
> +#include "coresight-tpda.h"
> +#include "coresight-trace-id.h"
> +
> +DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
> +
> +/* Settings pre enabling port control register */
> +static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
> +{
> + u32 val;
> +
> + val = readl_relaxed(drvdata->base + TPDA_CR);
> + /* Bits 6 ~ 12 is for atid value */

minor nit, please could we use FIELD_PREP() and define the
ATID field in the TPDA_CR accordingly ?

> + val |= (drvdata->atid << 6)

then we could have :

val |= FIELD_PREP(TPDA_CR_ATID, drvdata->atid);

> + writel_relaxed(val, drvdata->base + TPDA_CR);
> +}
> +
> +static void tpda_enable_port(struct tpda_drvdata *drvdata, int port)
> +{
> + u32 val;
> +
> + val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
> + /* Enable the port */
> + val |= TPDA_Pn_CR_ENA;
> + writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
> +}
> +
> +static void _tpda_enable(struct tpda_drvdata *drvdata, int port)

minor nit: Similar to the tpdm, comment please use __tpda_{enable,disable}()

> +{
> + CS_UNLOCK(drvdata->base);
> +
> + if (!drvdata->enable)
> + tpda_enable_pre_port(drvdata);
> +
> + tpda_enable_port(drvdata, port);
> +
> + CS_LOCK(drvdata->base);
> +}
> +
> +static int tpda_enable(struct coresight_device *csdev, int inport, int outport)
> +{
> + struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + mutex_lock(&drvdata->lock);
> + _tpda_enable(drvdata, inport);
> + drvdata->enable = true;

I am wondering if this is good enough ? Do we need a refcount ?
e.g, multiple TPDMs could be enabled independently and disabled
indpendently. And the tpda must stay alive until the last "source"
is gone away ?

> + mutex_unlock(&drvdata->lock);
> +
> + dev_info(drvdata->dev, "TPDA inport %d enabled\n", inport);
> + return 0;
> +}
> +
> +static void _tpda_disable(struct tpda_drvdata *drvdata, int port)
> +{
> + u32 val;
> +
> + CS_UNLOCK(drvdata->base);
> +
> + val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
> + val &= ~TPDA_Pn_CR_ENA;
> + writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
> +
> + CS_LOCK(drvdata->base);
> +}
> +
> +static void tpda_disable(struct coresight_device *csdev, int inport,
> + int outport)
> +{
> + struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + mutex_lock(&drvdata->lock);
> + _tpda_disable(drvdata, inport);
> + drvdata->enable = false;

This is not sufficient. We need to make sure the TPDA has a refcount of
the enabled ports.

> + mutex_unlock(&drvdata->lock);
> +
> + dev_info(drvdata->dev, "TPDA inport %d disabled\n", inport);
> +}
> +
> +static const struct coresight_ops_link tpda_link_ops = {
> + .enable = tpda_enable,
> + .disable = tpda_disable,
> +};
> +
> +static const struct coresight_ops tpda_cs_ops = {
> + .link_ops = &tpda_link_ops,
> +};
> +
> +static int tpda_init_default_data(struct tpda_drvdata *drvdata)
> +{
> + int atid;
> + /*
> + * TPDA must has a unique atid. This atid can uniquely
> + * identify the TPDM trace source connect to the TPDA.

nit: s/connect/connected/

Also how do we identify the different TPDM sources
connected using a single atid ? Looking at the patch description
it is possible to have multiple TPDMs connected to a single TPDA.

> + */
> + atid = coresight_trace_id_get_system_id(coresight_get_trace_id_map());
> + if (atid < 0)
> + return atid;
> +
> + drvdata->atid = atid;
> + return 0;
> +}
> +
> +static int tpda_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> + int ret;
> + struct device *dev = &adev->dev;
> + struct coresight_platform_data *pdata;
> + struct tpda_drvdata *drvdata;
> + struct coresight_desc desc = { 0 };
> +
> + pdata = coresight_get_platform_data(dev);
> + if (IS_ERR(pdata))
> + return PTR_ERR(pdata);
> + adev->dev.platform_data = pdata;
> +
> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + drvdata->dev = &adev->dev;
> + dev_set_drvdata(dev, drvdata);
> +
> + drvdata->base = devm_ioremap_resource(dev, &adev->res);
> + if (!drvdata->base)
> + return -ENOMEM;
> +
> + mutex_init(&drvdata->lock);
> +
> + ret = tpda_init_default_data(drvdata);
> + if (ret)
> + return ret;
> +
> + desc.name = coresight_alloc_device_name(&tpda_devs, dev);
> + if (!desc.name)
> + return -ENOMEM;
> + desc.type = CORESIGHT_DEV_TYPE_LINK;
> + desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
> + desc.ops = &tpda_cs_ops;
> + desc.pdata = adev->dev.platform_data;
> + desc.dev = &adev->dev;

desc.access must be initialised.

> + drvdata->csdev = coresight_register(&desc);
> + if (IS_ERR(drvdata->csdev))
> + return PTR_ERR(drvdata->csdev);
> +
> + pm_runtime_put(&adev->dev);
> +
> + dev_dbg(drvdata->dev, "TPDA initialized\n");
> + return 0;
> +}
> +
> +static void __exit tpda_remove(struct amba_device *adev)
> +{
> + struct tpda_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> + coresight_unregister(drvdata->csdev);
> +}
> +
> +/*
> + * Different TPDA has different periph id.
> + * The difference is 0-7 bits' value. So ignore 0-7 bits.
> + */
> +static struct amba_id tpda_ids[] = {
> + {
> + .id = 0x000f0f00,
> + .mask = 0x000fff00,
> + },
> + { 0, 0},
> +};
> +
> +static struct amba_driver tpda_driver = {
> + .drv = {
> + .name = "coresight-tpda",
> + .owner = THIS_MODULE,
> + .suppress_bind_attrs = true,
> + },
> + .probe = tpda_probe,
> + .remove = tpda_remove,
> + .id_table = tpda_ids,
> +};
> +
> +module_amba_driver(tpda_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Aggregator driver");
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h
> new file mode 100644
> index 000000000000..6df1b72b3b76
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef _CORESIGHT_CORESIGHT_TPDA_H
> +#define _CORESIGHT_CORESIGHT_TPDA_H
> +
> +#define TPDA_CR (0x000)
> +#define TPDA_Pn_CR(n) (0x004 + (n * 4))
> +/* Aggregator port enable bit */
> +#define TPDA_Pn_CR_ENA BIT(0)
> +
> +#define TPDA_MAX_INPORTS 32

Please add the bit fields for TPDA_CR_ATID here and use
the FIELD_PREP()

#define TPDA_CR_ATID GENMASK(12, 6)

> +
> +/**
> + * struct tpda_drvdata - specifics associated to an TPDA component
> + * @base: memory mapped base address for this component.
> + * @dev: The device entity associated to this component.
> + * @csdev: component vitals needed by the framework.
> + * @lock: lock for the enable value.
> + * @enable: enable status of the component.
> + */
> +struct tpda_drvdata {
> + void __iomem *base;
> + struct device *dev;
> + struct coresight_device *csdev;
> + struct mutex lock;

Why mutex and not spinlock ?

> + bool enable;
> + u32 atid;

u8 atid ?


Suzuki

2022-05-23 09:54:47

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v7 08/10] dt-bindings: arm: Adds CoreSight TPDA hardware definitions

Cc: Rob Herring



On 09/05/2022 14:39, Mao Jinlong wrote:
> Adds new coresight-tpda.yaml file describing the bindings required
> to define tpda in the device trees.
>
> Reviewed-by: Mike Leach <[email protected]>
> Signed-off-by: Tao Zhang <[email protected]>
> Signed-off-by: Mao Jinlong <[email protected]>
> ---
> .../bindings/arm/coresight-tpda.yaml | 119 ++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 120 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpda.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/coresight-tpda.yaml b/Documentation/devicetree/bindings/arm/coresight-tpda.yaml
> new file mode 100644
> index 000000000000..4948ac13e7f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/coresight-tpda.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +# Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/coresight-tpda.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Trace, Profiling and Diagnostics Aggregator - TPDA
> +
> +description: |
> + TPDAs are responsible for packetization and timestamping of data sets
> + utilizing the MIPI STPv2 packet protocol. Pulling data sets from one or
> + more attached TPDM and pushing the resultant (packetized) data out a
> + master ATB interface. Performing an arbitrated ATB interleaving (funneling)
> + task for free-flowing data from TPDM (i.e. CMB and DSB data set flows).
> +
> +maintainers:
> + - Mao Jinlong <[email protected]>
> + - Tao Zhang <[email protected]>
> +
> +properties:
> + $nodename:
> + pattern: "^tpda(@[0-9a-f]+)$"
> + compatible:
> + items:
> + - const: qcom,coresight-tpda
> + - const: arm,primecell
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: apb_pclk
> +
> + in-ports:
> + type: object
> + description: |
> + Input connections from TPDM to TPDA
> + $ref: /schemas/graph.yaml#/properties/ports
> +

--->8---
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + patternProperties:
> + "^port@[0-9a-f]+$":
> + type: object
> + required:
> + - reg
> +
> + required:
> + - '#size-cells'
> + - '#address-cells'

---8<---

I believe the above snippet is not needed and is covered by the generic
ports.


> +
> + out-ports:
> + type: object
> + description: |
> + Output connections from the TPDA to legacy CoreSight trace bus.
> + $ref: /schemas/graph.yaml#/properties/ports
> +
> + properties:
> + port:
> + description:
> + Output connection from the TPDA to legacy CoreSight Trace bus.
> + $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - in-ports
> + - out-ports
> +
> +additionalProperties: false
> +
> +examples:
> + # minimum tpda definition.
> + - |
> + tpda@6004000 {
> + compatible = "qcom,coresight-tpda", "arm,primecell";
> + reg = <0x6004000 0x1000>;
> +
> + qcom,tpda-atid = <65>;
> +
> + clocks = <&aoss_qmp>;
> + clock-names = "apb_pclk";
> +
> + in-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + tpda_qdss_0_in_tpdm_dcc: endpoint {
> + remote-endpoint =
> + <&tpdm_dcc_out_tpda_qdss_0>;
> + };
> + };
> + };
> +
> + out-ports {
> + port {
> + tpda_qdss_out_funnel_in0: endpoint {
> + remote-endpoint =
> + <&funnel_in0_in_tpda_qdss>;
> + };
> + };
> + };
> + };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 28d32b3f3f5c..5d2d8c0ee340 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1978,6 +1978,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git
> F: Documentation/ABI/testing/sysfs-bus-coresight-devices-*
> F: Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
> F: Documentation/devicetree/bindings/arm/coresight-cti.yaml
> +F: Documentation/devicetree/bindings/arm/coresight-tpda.yaml
> F: Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
> F: Documentation/devicetree/bindings/arm/coresight.txt
> F: Documentation/devicetree/bindings/arm/ete.yaml

Otherwise looks good to me.

Suzuki

2022-05-23 14:25:44

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v7 08/10] dt-bindings: arm: Adds CoreSight TPDA hardware definitions

On Mon, May 23, 2022 at 4:44 AM Suzuki K Poulose <[email protected]> wrote:
>
> Cc: Rob Herring
>

Will or will not have any effect...

Please use get_maintainers.pl and send your patches to the right
lists/maintainers. DT patches only get reviewed if sent to DT list. So
please resend to the DT list. But before you do, I can tell this
binding hasn't been tested so fix all the warnings first.

Rob

>
>
> On 09/05/2022 14:39, Mao Jinlong wrote:
> > Adds new coresight-tpda.yaml file describing the bindings required
> > to define tpda in the device trees.
> >
> > Reviewed-by: Mike Leach <[email protected]>
> > Signed-off-by: Tao Zhang <[email protected]>
> > Signed-off-by: Mao Jinlong <[email protected]>
> > ---
> > .../bindings/arm/coresight-tpda.yaml | 119 ++++++++++++++++++
> > MAINTAINERS | 1 +
> > 2 files changed, 120 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpda.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/arm/coresight-tpda.yaml b/Documentation/devicetree/bindings/arm/coresight-tpda.yaml
> > new file mode 100644
> > index 000000000000..4948ac13e7f8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/coresight-tpda.yaml
> > @@ -0,0 +1,119 @@
> > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> > +# Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/coresight-tpda.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Trace, Profiling and Diagnostics Aggregator - TPDA
> > +
> > +description: |
> > + TPDAs are responsible for packetization and timestamping of data sets
> > + utilizing the MIPI STPv2 packet protocol. Pulling data sets from one or
> > + more attached TPDM and pushing the resultant (packetized) data out a
> > + master ATB interface. Performing an arbitrated ATB interleaving (funneling)
> > + task for free-flowing data from TPDM (i.e. CMB and DSB data set flows).
> > +
> > +maintainers:
> > + - Mao Jinlong <[email protected]>
> > + - Tao Zhang <[email protected]>
> > +
> > +properties:
> > + $nodename:
> > + pattern: "^tpda(@[0-9a-f]+)$"
> > + compatible:
> > + items:
> > + - const: qcom,coresight-tpda
> > + - const: arm,primecell
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + items:
> > + - const: apb_pclk
> > +
> > + in-ports:
> > + type: object
> > + description: |
> > + Input connections from TPDM to TPDA
> > + $ref: /schemas/graph.yaml#/properties/ports
> > +
>
> --->8---
> > + properties:
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + patternProperties:
> > + "^port@[0-9a-f]+$":
> > + type: object
> > + required:
> > + - reg
> > +
> > + required:
> > + - '#size-cells'
> > + - '#address-cells'
>
> ---8<---
>
> I believe the above snippet is not needed and is covered by the generic
> ports.
>
>
> > +
> > + out-ports:
> > + type: object
> > + description: |
> > + Output connections from the TPDA to legacy CoreSight trace bus.
> > + $ref: /schemas/graph.yaml#/properties/ports
> > +
> > + properties:
> > + port:
> > + description:
> > + Output connection from the TPDA to legacy CoreSight Trace bus.
> > + $ref: /schemas/graph.yaml#/properties/port
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > + - in-ports
> > + - out-ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + # minimum tpda definition.
> > + - |
> > + tpda@6004000 {
> > + compatible = "qcom,coresight-tpda", "arm,primecell";
> > + reg = <0x6004000 0x1000>;
> > +
> > + qcom,tpda-atid = <65>;
> > +
> > + clocks = <&aoss_qmp>;
> > + clock-names = "apb_pclk";
> > +
> > + in-ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + tpda_qdss_0_in_tpdm_dcc: endpoint {
> > + remote-endpoint =
> > + <&tpdm_dcc_out_tpda_qdss_0>;
> > + };
> > + };
> > + };
> > +
> > + out-ports {
> > + port {
> > + tpda_qdss_out_funnel_in0: endpoint {
> > + remote-endpoint =
> > + <&funnel_in0_in_tpda_qdss>;
> > + };
> > + };
> > + };
> > + };
> > +
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 28d32b3f3f5c..5d2d8c0ee340 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1978,6 +1978,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git
> > F: Documentation/ABI/testing/sysfs-bus-coresight-devices-*
> > F: Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
> > F: Documentation/devicetree/bindings/arm/coresight-cti.yaml
> > +F: Documentation/devicetree/bindings/arm/coresight-tpda.yaml
> > F: Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
> > F: Documentation/devicetree/bindings/arm/coresight.txt
> > F: Documentation/devicetree/bindings/arm/ete.yaml
>
> Otherwise looks good to me.
>
> Suzuki

2022-05-24 09:08:17

by Mao Jinlong

[permalink] [raw]
Subject: Re: [PATCH v7 07/10] Coresight: Add TPDA link driver

Hi Suzuki,

On 5/23/2022 5:40 PM, Suzuki K Poulose wrote:
> On 09/05/2022 14:39, Mao Jinlong wrote:
>> TPDA(Trace, Profiling and Diagnostics Aggregator) is
>> to provide packetization, funneling and timestamping of
>> TPDM data. Multiple monitors are connected to different
>> input ports of TPDA.This change is to add tpda
>> enable/disable/probe functions for coresight tpda driver.
>>
>>   - - - -         - - - -        - - - -
>> | TPDM 0|      | TPDM 1 |     | TPDM 2|
>>   - - - -         - - - -        - - - -
>>      |               |             |
>>      |_ _ _ _ _ _    |     _ _ _ _ |
>>                  |   |    |
>>                  |   |    |
>>             ------------------
>>            |        TPDA      |
>>             ------------------
>>
>> Signed-off-by: Tao Zhang <[email protected]>
>> Signed-off-by: Mao Jinlong <[email protected]>
>> ---
>>   drivers/hwtracing/coresight/Kconfig          |  11 +
>>   drivers/hwtracing/coresight/Makefile         |   1 +
>>   drivers/hwtracing/coresight/coresight-tpda.c | 201 +++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tpda.h |  33 +++
>>   4 files changed, 246 insertions(+)
>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c
>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpda.h
>>
>> diff --git a/drivers/hwtracing/coresight/Kconfig
>> b/drivers/hwtracing/coresight/Kconfig
>> index 5c506a1cd08f..447919565326 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -205,6 +205,7 @@ config CORESIGHT_TRBE
>>   config CORESIGHT_TPDM
>>       tristate "CoreSight Trace, Profiling & Diagnostics Monitor driver"
>>       select CORESIGHT_LINKS_AND_SINKS
>> +    select CORESIGHT_TPDA
>>       help
>>         This driver provides support for configuring monitor.
>> Monitors are
>>         primarily responsible for data set collection and support the
>> @@ -214,4 +215,14 @@ config CORESIGHT_TPDM
>>         To compile this driver as a module, choose M here: the module
>> will be
>>         called coresight-tpdm.
>>   +config CORESIGHT_TPDA
>> +    tristate "CoreSight Trace, Profiling & Diagnostics Aggregator
>> driver"
>> +    help
>> +      This driver provides support for configuring aggregator. This is
>> +      primarily useful for pulling the data sets from one or more
>> +      attached monitors and pushing the resultant data out. Multiple
>> +      monitors are connected on different input ports of TPDA.
>> +
>> +      To compile this driver as a module, choose M here: the module
>> will be
>> +      called coresight-tpda.
>>   endif
>> diff --git a/drivers/hwtracing/coresight/Makefile
>> b/drivers/hwtracing/coresight/Makefile
>> index 6bb9b1746bc7..1712d82e7260 100644
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>> @@ -26,5 +26,6 @@ obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o
>>   obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
>>   obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o
>>   obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o
>> +obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o
>>   coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \
>>              coresight-cti-sysfs.o
>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c
>> b/drivers/hwtracing/coresight/coresight-tpda.c
>> new file mode 100644
>> index 000000000000..126286e89679
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>> @@ -0,0 +1,201 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights
>> reserved.
>> + */
>> +
>> +#include <linux/amba/bus.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/coresight.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/fs.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "coresight-priv.h"
>> +#include "coresight-tpda.h"
>> +#include "coresight-trace-id.h"
>> +
>> +DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
>> +
>> +/* Settings pre enabling port control register */
>> +static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
>> +{
>> +    u32 val;
>> +
>> +    val = readl_relaxed(drvdata->base + TPDA_CR);
>> +    /* Bits 6 ~ 12 is for atid value */
>
> minor nit, please could we use FIELD_PREP() and define the
> ATID field in the TPDA_CR accordingly ?
I will check and update.
>
>> +    val |= (drvdata->atid << 6)
>
> then we could have :
>
>     val |= FIELD_PREP(TPDA_CR_ATID, drvdata->atid);
>
>> +    writel_relaxed(val, drvdata->base + TPDA_CR);
>> +}
>> +
>> +static void tpda_enable_port(struct tpda_drvdata *drvdata, int port)
>> +{
>> +    u32 val;
>> +
>> +    val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
>> +    /* Enable the port */
>> +    val |= TPDA_Pn_CR_ENA;
>> +    writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
>> +}
>> +
>> +static void _tpda_enable(struct tpda_drvdata *drvdata, int port)
>
> minor nit: Similar to the tpdm, comment please use
> __tpda_{enable,disable}()
I will address this.
>
>> +{
>> +    CS_UNLOCK(drvdata->base);
>> +
>> +    if (!drvdata->enable)
>> +        tpda_enable_pre_port(drvdata);
>> +
>> +    tpda_enable_port(drvdata, port);
>> +
>> +    CS_LOCK(drvdata->base);
>> +}
>> +
>> +static int tpda_enable(struct coresight_device *csdev, int inport,
>> int outport)
>> +{
>> +    struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    mutex_lock(&drvdata->lock);
>> +    _tpda_enable(drvdata, inport);
>> +    drvdata->enable = true;
>
> I am wondering if this is good enough ? Do we need a refcount ?
> e.g, multiple TPDMs could be enabled independently and disabled
> indpendently. And the tpda must stay alive until the last "source"
> is gone away ?
it makes sense.
>
>> +    mutex_unlock(&drvdata->lock);
>> +
>> +    dev_info(drvdata->dev, "TPDA inport %d enabled\n", inport);
>> +    return 0;
>> +}
>> +
>> +static void _tpda_disable(struct tpda_drvdata *drvdata, int port)
>> +{
>> +    u32 val;
>> +
>> +    CS_UNLOCK(drvdata->base);
>> +
>> +    val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
>> +    val &= ~TPDA_Pn_CR_ENA;
>> +    writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
>> +
>> +    CS_LOCK(drvdata->base);
>> +}
>> +
>> +static void tpda_disable(struct coresight_device *csdev, int inport,
>> +               int outport)
>> +{
>> +    struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    mutex_lock(&drvdata->lock);
>> +    _tpda_disable(drvdata, inport);
>> +    drvdata->enable = false;
>
> This is not sufficient. We need to make sure the TPDA has a refcount of
> the enabled ports.
I will update.
>
>> +    mutex_unlock(&drvdata->lock);
>> +
>> +    dev_info(drvdata->dev, "TPDA inport %d disabled\n", inport);
>> +}
>> +
>> +static const struct coresight_ops_link tpda_link_ops = {
>> +    .enable        = tpda_enable,
>> +    .disable    = tpda_disable,
>> +};
>> +
>> +static const struct coresight_ops tpda_cs_ops = {
>> +    .link_ops    = &tpda_link_ops,
>> +};
>> +
>> +static int tpda_init_default_data(struct tpda_drvdata *drvdata)
>> +{
>> +    int atid;
>> +    /*
>> +     * TPDA must has a unique atid. This atid can uniquely
>> +     * identify the TPDM trace source connect to the TPDA.
>
> nit: s/connect/connected/
>
> Also how do we identify the different TPDM sources
> connected using a single atid ? Looking at the patch description
> it is possible to have multiple TPDMs connected to a single TPDA.
>
>> +     */
>> +    atid =
>> coresight_trace_id_get_system_id(coresight_get_trace_id_map());
>> +    if (atid < 0)
>> +        return atid;
>> +
>> +    drvdata->atid = atid;
>> +    return 0;
>> +}
>> +
>> +static int tpda_probe(struct amba_device *adev, const struct amba_id
>> *id)
>> +{
>> +    int ret;
>> +    struct device *dev = &adev->dev;
>> +    struct coresight_platform_data *pdata;
>> +    struct tpda_drvdata *drvdata;
>> +    struct coresight_desc desc = { 0 };
>> +
>> +    pdata = coresight_get_platform_data(dev);
>> +    if (IS_ERR(pdata))
>> +        return PTR_ERR(pdata);
>> +    adev->dev.platform_data = pdata;
>> +
>> +    drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> +    if (!drvdata)
>> +        return -ENOMEM;
>> +
>> +    drvdata->dev = &adev->dev;
>> +    dev_set_drvdata(dev, drvdata);
>> +
>> +    drvdata->base = devm_ioremap_resource(dev, &adev->res);
>> +    if (!drvdata->base)
>> +        return -ENOMEM;
>> +
>> +    mutex_init(&drvdata->lock);
>> +
>> +    ret = tpda_init_default_data(drvdata);
>> +    if (ret)
>> +        return ret;
>> +
>> +    desc.name = coresight_alloc_device_name(&tpda_devs, dev);
>> +    if (!desc.name)
>> +        return -ENOMEM;
>> +    desc.type = CORESIGHT_DEV_TYPE_LINK;
>> +    desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
>> +    desc.ops = &tpda_cs_ops;
>> +    desc.pdata = adev->dev.platform_data;
>> +    desc.dev = &adev->dev;
>
> desc.access must be initialised.
I will address it.
>
>> +    drvdata->csdev = coresight_register(&desc);
>> +    if (IS_ERR(drvdata->csdev))
>> +        return PTR_ERR(drvdata->csdev);
>> +
>> +    pm_runtime_put(&adev->dev);
>> +
>> +    dev_dbg(drvdata->dev, "TPDA initialized\n");
>> +    return 0;
>> +}
>> +
>> +static void __exit tpda_remove(struct amba_device *adev)
>> +{
>> +    struct tpda_drvdata *drvdata = dev_get_drvdata(&adev->dev);
>> +
>> +    coresight_unregister(drvdata->csdev);
>> +}
>> +
>> +/*
>> + * Different TPDA has different periph id.
>> + * The difference is 0-7 bits' value. So ignore 0-7 bits.
>> + */
>> +static struct amba_id tpda_ids[] = {
>> +    {
>> +        .id     = 0x000f0f00,
>> +        .mask   = 0x000fff00,
>> +    },
>> +    { 0, 0},
>> +};
>> +
>> +static struct amba_driver tpda_driver = {
>> +    .drv = {
>> +        .name   = "coresight-tpda",
>> +        .owner    = THIS_MODULE,
>> +        .suppress_bind_attrs = true,
>> +    },
>> +    .probe          = tpda_probe,
>> +    .remove        = tpda_remove,
>> +    .id_table    = tpda_ids,
>> +};
>> +
>> +module_amba_driver(tpda_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Aggregator driver");
>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h
>> b/drivers/hwtracing/coresight/coresight-tpda.h
>> new file mode 100644
>> index 000000000000..6df1b72b3b76
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
>> @@ -0,0 +1,33 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights
>> reserved.
>> + */
>> +
>> +#ifndef _CORESIGHT_CORESIGHT_TPDA_H
>> +#define _CORESIGHT_CORESIGHT_TPDA_H
>> +
>> +#define TPDA_CR            (0x000)
>> +#define TPDA_Pn_CR(n)        (0x004 + (n * 4))
>> +/* Aggregator port enable bit */
>> +#define TPDA_Pn_CR_ENA        BIT(0)
>> +
>> +#define TPDA_MAX_INPORTS    32
>
> Please add the bit fields for  TPDA_CR_ATID here and use
> the FIELD_PREP()
>
> #define TPDA_CR_ATID            GENMASK(12, 6)
I will check and update.
>
>> +
>> +/**
>> + * struct tpda_drvdata - specifics associated to an TPDA component
>> + * @base:       memory mapped base address for this component.
>> + * @dev:        The device entity associated to this component.
>> + * @csdev:      component vitals needed by the framework.
>> + * @lock:       lock for the enable value.
>> + * @enable:     enable status of the component.
>> + */
>> +struct tpda_drvdata {
>> +    void __iomem        *base;
>> +    struct device        *dev;
>> +    struct coresight_device    *csdev;
>> +    struct mutex        lock;
>
> Why mutex and not spinlock ?

Same as tpdm.

1. There is no irq for TPDA
2. There will be many registers to configure during enable/disable which
may cause
some time.
>
>> +    bool            enable;
>> +    u32            atid;
>
>     u8 atid ?
I will check.
>
>
> Suzuki

2022-05-24 11:40:22

by Mao Jinlong

[permalink] [raw]
Subject: Re: [PATCH v7 08/10] dt-bindings: arm: Adds CoreSight TPDA hardware definitions

Thanks Rob and Suzuki for the review.

I will fix the warning and update in next version.

On 5/23/2022 10:24 PM, Rob Herring wrote:
> On Mon, May 23, 2022 at 4:44 AM Suzuki K Poulose <[email protected]> wrote:
>> Cc: Rob Herring
>>
> Will or will not have any effect...
>
> Please use get_maintainers.pl and send your patches to the right
> lists/maintainers. DT patches only get reviewed if sent to DT list. So
> please resend to the DT list. But before you do, I can tell this
> binding hasn't been tested so fix all the warnings first.
>
> Rob
>
>>
>> On 09/05/2022 14:39, Mao Jinlong wrote:
>>> Adds new coresight-tpda.yaml file describing the bindings required
>>> to define tpda in the device trees.
>>>
>>> Reviewed-by: Mike Leach <[email protected]>
>>> Signed-off-by: Tao Zhang <[email protected]>
>>> Signed-off-by: Mao Jinlong <[email protected]>
>>> ---
>>> .../bindings/arm/coresight-tpda.yaml | 119 ++++++++++++++++++
>>> MAINTAINERS | 1 +
>>> 2 files changed, 120 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpda.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/coresight-tpda.yaml b/Documentation/devicetree/bindings/arm/coresight-tpda.yaml
>>> new file mode 100644
>>> index 000000000000..4948ac13e7f8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/coresight-tpda.yaml
>>> @@ -0,0 +1,119 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
>>> +# Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/arm/coresight-tpda.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Trace, Profiling and Diagnostics Aggregator - TPDA
>>> +
>>> +description: |
>>> + TPDAs are responsible for packetization and timestamping of data sets
>>> + utilizing the MIPI STPv2 packet protocol. Pulling data sets from one or
>>> + more attached TPDM and pushing the resultant (packetized) data out a
>>> + master ATB interface. Performing an arbitrated ATB interleaving (funneling)
>>> + task for free-flowing data from TPDM (i.e. CMB and DSB data set flows).
>>> +
>>> +maintainers:
>>> + - Mao Jinlong <[email protected]>
>>> + - Tao Zhang <[email protected]>
>>> +
>>> +properties:
>>> + $nodename:
>>> + pattern: "^tpda(@[0-9a-f]+)$"
>>> + compatible:
>>> + items:
>>> + - const: qcom,coresight-tpda
>>> + - const: arm,primecell
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + clocks:
>>> + maxItems: 1
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: apb_pclk
>>> +
>>> + in-ports:
>>> + type: object
>>> + description: |
>>> + Input connections from TPDM to TPDA
>>> + $ref: /schemas/graph.yaml#/properties/ports
>>> +
>> --->8---
>>> + properties:
>>> + '#address-cells':
>>> + const: 1
>>> +
>>> + '#size-cells':
>>> + const: 0
>>> +
>>> + patternProperties:
>>> + "^port@[0-9a-f]+$":
>>> + type: object
>>> + required:
>>> + - reg
>>> +
>>> + required:
>>> + - '#size-cells'
>>> + - '#address-cells'
>> ---8<---
>>
>> I believe the above snippet is not needed and is covered by the generic
>> ports.
>>
>>
>>> +
>>> + out-ports:
>>> + type: object
>>> + description: |
>>> + Output connections from the TPDA to legacy CoreSight trace bus.
>>> + $ref: /schemas/graph.yaml#/properties/ports
>>> +
>>> + properties:
>>> + port:
>>> + description:
>>> + Output connection from the TPDA to legacy CoreSight Trace bus.
>>> + $ref: /schemas/graph.yaml#/properties/port
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - clocks
>>> + - clock-names
>>> + - in-ports
>>> + - out-ports
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + # minimum tpda definition.
>>> + - |
>>> + tpda@6004000 {
>>> + compatible = "qcom,coresight-tpda", "arm,primecell";
>>> + reg = <0x6004000 0x1000>;
>>> +
>>> + qcom,tpda-atid = <65>;
>>> +
>>> + clocks = <&aoss_qmp>;
>>> + clock-names = "apb_pclk";
>>> +
>>> + in-ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>> + reg = <0>;
>>> + tpda_qdss_0_in_tpdm_dcc: endpoint {
>>> + remote-endpoint =
>>> + <&tpdm_dcc_out_tpda_qdss_0>;
>>> + };
>>> + };
>>> + };
>>> +
>>> + out-ports {
>>> + port {
>>> + tpda_qdss_out_funnel_in0: endpoint {
>>> + remote-endpoint =
>>> + <&funnel_in0_in_tpda_qdss>;
>>> + };
>>> + };
>>> + };
>>> + };
>>> +
>>> +...
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 28d32b3f3f5c..5d2d8c0ee340 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1978,6 +1978,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git
>>> F: Documentation/ABI/testing/sysfs-bus-coresight-devices-*
>>> F: Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
>>> F: Documentation/devicetree/bindings/arm/coresight-cti.yaml
>>> +F: Documentation/devicetree/bindings/arm/coresight-tpda.yaml
>>> F: Documentation/devicetree/bindings/arm/coresight-tpdm.yaml
>>> F: Documentation/devicetree/bindings/arm/coresight.txt
>>> F: Documentation/devicetree/bindings/arm/ete.yaml
>> Otherwise looks good to me.
>>
>> Suzuki

2022-05-24 11:43:09

by Mao Jinlong

[permalink] [raw]
Subject: Re: [PATCH v7 05/10] coresight-tpdm: Add integration test support

Good afternoon.

On 5/23/2022 5:17 PM, Suzuki K Poulose wrote:
> On 09/05/2022 14:39, Mao Jinlong wrote:
>> Integration test for tpdm can help to generate the data for
>> verification of the topology during TPDM software bring up.
>>
>> Sample:
>> echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink
>> echo 1 > /sys/bus/coresight/devices/tpdm0/enable_source
>> echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test
>> echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test
>> cat /dev/tmc_etf0 > /data/etf-tpdm0.bin
>>
>> Signed-off-by: Tao Zhang <[email protected]>
>> Signed-off-by: Mao Jinlong <[email protected]>
>
> Please could we stick this under a sub-Kconfig entry, like
> we did for the CTI ?
>
> CONFIG_CORESIGHT_TPMD_INTEGRATION_TEST
This is comment in V5 patch. There is no adverse effects for all tpdms with
integration test enabled. So i removed this config.

"

For the last patchset you mentioned that you were making this
configurable because the CTI intgration tests were also configurable.
The reason that the CTI intergration test registers were done in this
way is that some of the CoreSight components were not guaranteed to
return to a usable state once integration test was disabled.
Thus after use of the integration test, a complete board reset was
recommended. Therefore we ensured that these features would only be
used by those specifically configuring them and who were hopefully
aware of the potentail limitations

If your hardware can reliably enable and disable integration test
without adverse effects, then you may wish to consider making the
integration test a permanent feature of the driver.

Regards

Mike
"


>
>> ---
>>   drivers/hwtracing/coresight/coresight-tpdm.c | 54 ++++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tpdm.h | 14 +++++
>>   2 files changed, 68 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c
>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>> index 70df888ac565..57e38aa7d2bd 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -123,6 +123,59 @@ static void tpdm_init_default_data(struct
>> tpdm_drvdata *drvdata)
>>       CS_LOCK(drvdata->base);
>>   }
>>   +/*
>> + * value 1: 64 bits test data
>> + * value 2: 32 bits test data
>> + */
>> +static ssize_t integration_test_store(struct device *dev,
>> +                      struct device_attribute *attr,
>> +                      const char *buf,
>> +                      size_t size)
>> +{
>> +    int i, ret = 0;
>> +    unsigned long val;
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +    ret = kstrtoul(buf, 10, &val);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (val != 1 && val != 2)
>> +        return -EINVAL;
>> +
>> +    if (!drvdata->enable)
>> +        return -EINVAL;
>> +
>> +    if (val == 1)
>> +        val = ATBCNTRL_VAL_64;
>> +    else
>> +        val = ATBCNTRL_VAL_32;
>> +    CS_UNLOCK(drvdata->base);
>> +    writel_relaxed(0x1, drvdata->base + TPDM_ITCNTRL);
>> +
>> +    for (i = 1; i < INTEGRATION_TEST_CYCLE; i++)
>
> super minor nit : It is a bit un-natural, not to have i = 0;
This should be 0. I will update.
>
> Rest looks fine to me .
>
> Suzuki
>
>
>> +        writel_relaxed(val, drvdata->base + TPDM_ITATBCNTRL);
>> +
>> +    writel_relaxed(0, drvdata->base + TPDM_ITCNTRL);
>> +    CS_LOCK(drvdata->base);
>> +    return size;
>> +}
>> +static DEVICE_ATTR_WO(integration_test);
>> +
>> +static struct attribute *tpdm_attrs[] = {
>> +    &dev_attr_integration_test.attr,
>> +    NULL,
>> +};
>> +
>> +static struct attribute_group tpdm_attr_grp = {
>> +    .attrs = tpdm_attrs,
>> +};
>> +
>> +static const struct attribute_group *tpdm_attr_grps[] = {
>> +    &tpdm_attr_grp,
>> +    NULL,
>> +};
>> +
>>   static int tpdm_probe(struct amba_device *adev, const struct
>> amba_id *id)
>>   {
>>       struct device *dev = &adev->dev;
>> @@ -157,6 +210,7 @@ static int tpdm_probe(struct amba_device *adev,
>> const struct amba_id *id)
>>       desc.ops = &tpdm_cs_ops;
>>       desc.pdata = adev->dev.platform_data;
>>       desc.dev = &adev->dev;
>> +    desc.groups = tpdm_attr_grps;
>>       drvdata->csdev = coresight_register(&desc);
>>       if (IS_ERR(drvdata->csdev))
>>           return PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h
>> b/drivers/hwtracing/coresight/coresight-tpdm.h
>> index f95aaad9c653..4aa880794383 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
>> @@ -14,6 +14,20 @@
>>   /* Enable bit for DSB subunit */
>>   #define TPDM_DSB_CR_ENA        BIT(0)
>>   +/* TPDM integration test registers */
>> +#define TPDM_ITATBCNTRL        (0xEF0)
>> +#define TPDM_ITCNTRL        (0xF00)
>> +
>> +/* Register value for integration test */
>> +#define ATBCNTRL_VAL_32        0xC00F1409
>> +#define ATBCNTRL_VAL_64        0xC01F1409
>> +
>> +/*
>> + * Number of cycles to write value when
>> + * integration test.
>> + */
>> +#define INTEGRATION_TEST_CYCLE    10
>> +
>>   /**
>>    * This enum is for PERIPHIDR0 register of TPDM.
>>    * The fields [6:0] of PERIPHIDR0 are used to determine what
>
> _______________________________________________
> CoreSight mailing list -- [email protected]
> To unsubscribe send an email to [email protected]

2022-05-24 12:25:29

by Mao Jinlong

[permalink] [raw]
Subject: Re: [PATCH v7 02/10] Coresight: Add coresight TPDM source driver

Hi Suzuki,

Thank you for the review.

On 5/23/2022 4:57 PM, Suzuki K Poulose wrote:
> Hi
>
> On 09/05/2022 14:39, Mao Jinlong wrote:
>> Add driver to support Coresight device TPDM (Trace, Profiling and
>> Diagnostics Monitor). TPDM is a monitor to collect data from
>> different datasets. This change is to add probe/enable/disable
>> functions for tpdm source.
>>
>> Signed-off-by: Tao Zhang <[email protected]>
>> Signed-off-by: Mao Jinlong <[email protected]>
>> ---
>>   drivers/hwtracing/coresight/Kconfig          |  13 ++
>>   drivers/hwtracing/coresight/Makefile         |   1 +
>>   drivers/hwtracing/coresight/coresight-core.c |   5 +-
>>   drivers/hwtracing/coresight/coresight-tpdm.c | 146 +++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tpdm.h |  26 ++++
>>   include/linux/coresight.h                    |   1 +
>>   6 files changed, 191 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c
>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h
>>
>> diff --git a/drivers/hwtracing/coresight/Kconfig
>> b/drivers/hwtracing/coresight/Kconfig
>> index 514a9b8086e3..5c506a1cd08f 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -201,4 +201,17 @@ config CORESIGHT_TRBE
>>           To compile this driver as a module, choose M here: the
>> module will be
>>         called coresight-trbe.
>> +
>> +config CORESIGHT_TPDM
>> +    tristate "CoreSight Trace, Profiling & Diagnostics Monitor driver"
>> +    select CORESIGHT_LINKS_AND_SINKS
>> +    help
>> +      This driver provides support for configuring monitor. Monitors
>> are
>> +      primarily responsible for data set collection and support the
>> +      ability to collect any permutation of data set types. Monitors
>> are
>> +      also responsible for interaction with system cross triggering.
>
> I find the last statement a bit confusing. Could this be :
>
>     "Monitors are also connected to the cross triggers."
Some tpdm events can be triggered by CTI trigger out.
>
>> +
>> +      To compile this driver as a module, choose M here: the module
>> will be
>> +      called coresight-tpdm.
>> +
>>   endif
>> diff --git a/drivers/hwtracing/coresight/Makefile
>> b/drivers/hwtracing/coresight/Makefile
>> index 329a0c704b87..6bb9b1746bc7 100644
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>> @@ -25,5 +25,6 @@ obj-$(CONFIG_CORESIGHT_CPU_DEBUG) +=
>> coresight-cpu-debug.o
>>   obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o
>>   obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
>>   obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o
>> +obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o
>>   coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \
>>              coresight-cti-sysfs.o
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>> b/drivers/hwtracing/coresight/coresight-core.c
>> index 23ab16dd9b5d..75fe1781df20 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -1047,7 +1047,8 @@ static int coresight_validate_source(struct
>> coresight_device *csdev,
>>       }
>>         if (subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_PROC &&
>> -        subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE) {
>> +        subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE &&
>> +        subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY) {
>>           dev_err(&csdev->dev, "wrong device subtype in %s\n",
>> function);
>>           return -EINVAL;
>>       }
>> @@ -1116,6 +1117,7 @@ int coresight_enable(struct coresight_device
>> *csdev)
>>           per_cpu(tracer_path, cpu) = path;
>>           break;
>>       case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
>> +    case CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY:
>>           /*
>>            * Use the hash of source's device name as ID
>>            * and map the ID to the pointer of the path.
>> @@ -1165,6 +1167,7 @@ void coresight_disable(struct coresight_device
>> *csdev)
>>           per_cpu(tracer_path, cpu) = NULL;
>>           break;
>>       case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
>> +    case CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY:
>>           hash = hashlen_hash(hashlen_string(NULL,
>> dev_name(&csdev->dev)));
>>           /* Find the path by the hash. */
>>           path = idr_find(&path_idr, hash);
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c
>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>> new file mode 100644
>> index 000000000000..6a4e2a35053d
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -0,0 +1,146 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights
>> reserved.
>> + */
>> +
>> +#include <linux/amba/bus.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/coresight.h>
>> +#include <linux/coresight-pmu.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/fs.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +
>> +#include "coresight-priv.h"
>> +#include "coresight-tpdm.h"
>> +
>> +DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>> +
>> +/* TPDM enable operations */
>> +static int tpdm_enable(struct coresight_device *csdev,
>> +               struct perf_event *event, u32 mode)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    mutex_lock(&drvdata->lock);
>> +    if (drvdata->enable) {
>> +        mutex_unlock(&drvdata->lock);
>> +        return -EBUSY;
>> +    }
>> +
>> +    drvdata->enable = true;
>> +    mutex_unlock(&drvdata->lock);
>> +
>> +    dev_info(drvdata->dev, "TPDM tracing enabled\n");
>> +    return 0;
>> +}
>> +
>> +/* TPDM disable operations */
>> +static void tpdm_disable(struct coresight_device *csdev,
>> +             struct perf_event *event)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    mutex_lock(&drvdata->lock);
>> +    if (!drvdata->enable) {
>> +        mutex_unlock(&drvdata->lock);
>> +        return;
>> +    }
>> +
>> +    drvdata->enable = false;
>> +    mutex_unlock(&drvdata->lock);
>> +
>> +    dev_info(drvdata->dev, "TPDM tracing disabled\n");
>> +}
>> +
>> +static const struct coresight_ops_source tpdm_source_ops = {
>> +    .enable        = tpdm_enable,
>> +    .disable    = tpdm_disable,
>> +};
>> +
>> +static const struct coresight_ops tpdm_cs_ops = {
>> +    .source_ops    = &tpdm_source_ops,
>> +};
>> +
>> +static int tpdm_probe(struct amba_device *adev, const struct amba_id
>> *id)
>> +{
>> +    struct device *dev = &adev->dev;
>> +    struct coresight_platform_data *pdata;
>> +    struct tpdm_drvdata *drvdata;
>> +    struct coresight_desc desc = { 0 };
>> +
>> +    pdata = coresight_get_platform_data(dev);
>> +    if (IS_ERR(pdata))
>> +        return PTR_ERR(pdata);
>> +    adev->dev.platform_data = pdata;
>> +
>> +    /* driver data*/
>> +    drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> +    if (!drvdata)
>> +        return -ENOMEM;
>> +    drvdata->dev = &adev->dev;
>> +    dev_set_drvdata(dev, drvdata);
>> +
>> +    drvdata->base = devm_ioremap_resource(dev, &adev->res);
>> +    if (!drvdata->base)
>> +        return -ENOMEM;
>> +
>> +    mutex_init(&drvdata->lock);
>> +
>> +    /* Set up coresight component description */
>> +    desc.name = coresight_alloc_device_name(&tpdm_devs, dev);
>> +    if (!desc.name)
>> +        return -ENOMEM;
>> +    desc.type = CORESIGHT_DEV_TYPE_SOURCE;
>> +    desc.subtype.source_subtype =
>> CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY;
>> +    desc.ops = &tpdm_cs_ops;
>> +    desc.pdata = adev->dev.platform_data;
>> +    desc.dev = &adev->dev;
>
> desc.access must be initialised here.
>
>     desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
>
>> +    drvdata->csdev = coresight_register(&desc);
>> +    if (IS_ERR(drvdata->csdev))
>> +        return PTR_ERR(drvdata->csdev);
>> +
>> +    /* Decrease pm refcount when probe is done.*/
>> +    pm_runtime_put(&adev->dev);
>> +
>> +    return 0;
>> +}
>> +
>> +static void __exit tpdm_remove(struct amba_device *adev)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
>> +
>> +    coresight_unregister(drvdata->csdev);
>> +}
>> +
>> +/*
>> + * Different TPDM has different periph id.
>> + * The difference is 0-7 bits' value. So ignore 0-7 bits.
>> + */
>> +static struct amba_id tpdm_ids[] = {
>> +    {
>> +        .id = 0x000f0e00,
>> +        .mask = 0x000fff00,
>> +    },
>> +    { 0, 0},
>> +};
>> +
>> +static struct amba_driver tpdm_driver = {
>> +    .drv = {
>> +        .name   = "coresight-tpdm",
>> +        .owner    = THIS_MODULE,
>> +        .suppress_bind_attrs = true,
>> +    },
>> +    .probe          = tpdm_probe,
>> +    .id_table    = tpdm_ids,
>> +    .remove        = tpdm_remove,
>> +};
>> +
>> +module_amba_driver(tpdm_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Monitor driver");
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h
>> b/drivers/hwtracing/coresight/coresight-tpdm.h
>> new file mode 100644
>> index 000000000000..94a7748a5426
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights
>> reserved.
>> + */
>> +
>> +#ifndef _CORESIGHT_CORESIGHT_TPDM_H
>> +#define _CORESIGHT_CORESIGHT_TPDM_H
>> +
>> +/**
>> + * struct tpdm_drvdata - specifics associated to an TPDM component
>> + * @base:       memory mapped base address for this component.
>> + * @dev:        The device entity associated to this component.
>> + * @csdev:      component vitals needed by the framework.
>> + * @lock:       lock for the enable value.
>> + * @enable:     enable status of the component.
>> + */
>> +
>> +struct tpdm_drvdata {
>> +    void __iomem        *base;
>> +    struct device        *dev;
>> +    struct coresight_device    *csdev;
>> +    struct mutex        lock;
>
> Why mutex lock ? Couldn't this be a spinlock ?
1. There is no irq for TPDM
2. As there are 7 dataset types, there will be some FOR loop to configure
tpdm registers which may cause some time.


>
>> +    bool            enable;
>> +};
>> +
>> +#endif  /* _CORESIGHT_CORESIGHT_TPDM_H */
>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>> index 247147c11231..a9efac55029d 100644
>> --- a/include/linux/coresight.h
>> +++ b/include/linux/coresight.h
>> @@ -61,6 +61,7 @@ enum coresight_dev_subtype_source {
>>       CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
>>       CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
>>       CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
>> +    CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY,
>
> super minor nit: I find the choice of name a bit odd.
> We could simply make it something like :
>
>     CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS:
>
> Suzuki
I will check and update.
>
>>   };
>>     enum coresight_dev_subtype_helper {
>
> _______________________________________________
> CoreSight mailing list -- [email protected]
> To unsubscribe send an email to [email protected]

2022-05-24 16:03:11

by Mao Jinlong

[permalink] [raw]
Subject: Re: [PATCH v7 04/10] coresight-tpdm: Add DSB dataset support

Hi Suzuki,

On 5/23/2022 5:11 PM, Suzuki K Poulose wrote:
> Hi
>
> On 09/05/2022 14:39, Mao Jinlong wrote:
>> TPDM serves as data collection component for various dataset types.
>> DSB(Discrete Single Bit) is one of the dataset types. DSB subunit
>> can be enabled for data collection by writing 1 to the first bit of
>> DSB_CR register. This change is to add enable/disable function for
>> DSB dataset by writing DSB_CR register.
>
> The patch looks good to me, except for some minor comment below.
>
>>
>> Signed-off-by: Tao Zhang <[email protected]>
>> Signed-off-by: Mao Jinlong <[email protected]>
>> ---
>>   drivers/hwtracing/coresight/coresight-tpdm.c | 58 ++++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tpdm.h | 23 ++++++++
>>   2 files changed, 81 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c
>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>> index 6a4e2a35053d..70df888ac565 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -20,7 +20,28 @@
>>     DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>>   +static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>> +{
>> +    u32 val;
>> +
>> +    /* Set the enable bit of DSB control register to 1 */
>> +    val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
>> +    val |= TPDM_DSB_CR_ENA;
>> +    writel_relaxed(val, drvdata->base + TPDM_DSB_CR);
>> +}
>> +
>>   /* TPDM enable operations */
>> +static void _tpdm_enable(struct tpdm_drvdata *drvdata)
>> +{
>> +    CS_UNLOCK(drvdata->base);
>> +
>> +    /* Check if DSB datasets is present for TPDM. */
>> +    if (drvdata->datasets & BIT(TPDM_DS_DSB))
>> +        tpdm_enable_dsb(drvdata);
>> +
>> +    CS_LOCK(drvdata->base);
>> +}
>> +
>>   static int tpdm_enable(struct coresight_device *csdev,
>>                  struct perf_event *event, u32 mode)
>>   {
>> @@ -32,6 +53,7 @@ static int tpdm_enable(struct coresight_device *csdev,
>>           return -EBUSY;
>>       }
>>   +    _tpdm_enable(drvdata);
>>       drvdata->enable = true;
>>       mutex_unlock(&drvdata->lock);
>>   @@ -39,7 +61,29 @@ static int tpdm_enable(struct coresight_device
>> *csdev,
>>       return 0;
>>   }
>>   +static void tpdm_disable_dsb(struct tpdm_drvdata *drvdata)
>> +{
>> +    u32 val;
>> +
>> +    /* Set the enable bit of DSB control register to 0 */
>> +    val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
>> +    val &= ~TPDM_DSB_CR_ENA;
>> +    writel_relaxed(val, drvdata->base + TPDM_DSB_CR);
>> +}
>> +
>>   /* TPDM disable operations */
>> +static void _tpdm_disable(struct tpdm_drvdata *drvdata)
>> +{
>> +    CS_UNLOCK(drvdata->base);
>> +
>> +    /* Check if DSB datasets is present for TPDM. */
>> +    if (drvdata->datasets & BIT(TPDM_DS_DSB))
>> +        tpdm_disable_dsb(drvdata);
>> +
>> +    CS_LOCK(drvdata->base);
>> +
>
> nit: extra new line.
I will remove it.
>> +}
>> +
>>   static void tpdm_disable(struct coresight_device *csdev,
>>                struct perf_event *event)
>>   {
>> @@ -51,6 +95,7 @@ static void tpdm_disable(struct coresight_device
>> *csdev,
>>           return;
>>       }
>>   +    _tpdm_disable(drvdata);
>>       drvdata->enable = false;
>>       mutex_unlock(&drvdata->lock);
>>   @@ -66,6 +111,18 @@ static const struct coresight_ops tpdm_cs_ops = {
>>       .source_ops    = &tpdm_source_ops,
>>   };
>>   +static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
>> +{
>> +    int i;
>> +    u32 pidr;
>> +
>> +    CS_UNLOCK(drvdata->base);
>> +    /*  Get the datasets present on the TPDM. */
>> +    pidr = readl_relaxed(drvdata->base + CORESIGHT_PERIPHIDR0);
>> +    drvdata->datasets |= pidr & GENMASK(TPDM_DATASETS - 1, 0);
>> +    CS_LOCK(drvdata->base);
>> +}
>> +
>>   static int tpdm_probe(struct amba_device *adev, const struct
>> amba_id *id)
>>   {
>>       struct device *dev = &adev->dev;
>> @@ -104,6 +161,7 @@ static int tpdm_probe(struct amba_device *adev,
>> const struct amba_id *id)
>>       if (IS_ERR(drvdata->csdev))
>>           return PTR_ERR(drvdata->csdev);
>>   +    tpdm_init_default_data(drvdata);
>>       /* Decrease pm refcount when probe is done.*/
>>       pm_runtime_put(&adev->dev);
>>   diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h
>> b/drivers/hwtracing/coresight/coresight-tpdm.h
>> index 94a7748a5426..f95aaad9c653 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
>> @@ -6,6 +6,27 @@
>>   #ifndef _CORESIGHT_CORESIGHT_TPDM_H
>>   #define _CORESIGHT_CORESIGHT_TPDM_H
>>   +/* The max number of the datasets that TPDM supports */
>> +#define TPDM_DATASETS       7
>> +
>> +/* DSB Subunit Registers */
>> +#define TPDM_DSB_CR        (0x780)
>> +/* Enable bit for DSB subunit */
>> +#define TPDM_DSB_CR_ENA        BIT(0)
>> +
>> +/**
>> + * This enum is for PERIPHIDR0 register of TPDM.
>> + * The fields [6:0] of PERIPHIDR0 are used to determine what
>> + * interfaces and subunits are present on a given TPDM.
>> + *
>> + * PERIPHIDR0[0] : Fix to 1 if ImplDef subunit present, else 0
>> + * PERIPHIDR0[1] : Fix to 1 if DSB subunit present, else 0
>> + */
>> +enum tpdm_dataset {
>> +    TPDM_DS_IMPLDEF,
>> +    TPDM_DS_DSB,
>> +};
>
> Please could we name this explicitly to indicate the register field
> they appear in ? e.g,
>
> #define TPDM_PIDR0_DS_IMPDEF    BIT(0)
> #define    TPDM_PIDR0_DS_DSB    BIT(1)
I will check and update.
>
> Suzuki

2022-05-25 07:47:18

by Mao Jinlong

[permalink] [raw]
Subject: Re: [PATCH v7 04/10] coresight-tpdm: Add DSB dataset support

Hi Suzuki,

On 5/23/2022 5:24 PM, Suzuki K Poulose wrote:
> On 23/05/2022 10:11, Suzuki K Poulose wrote:
>> Hi
>>
>> On 09/05/2022 14:39, Mao Jinlong wrote:
>>> TPDM serves as data collection component for various dataset types.
>>> DSB(Discrete Single Bit) is one of the dataset types. DSB subunit
>>> can be enabled for data collection by writing 1 to the first bit of
>>> DSB_CR register. This change is to add enable/disable function for
>>> DSB dataset by writing DSB_CR register.
>>
>> The patch looks good to me, except for some minor comment below.
>>
>>>
>>> Signed-off-by: Tao Zhang <[email protected]>
>>> Signed-off-by: Mao Jinlong <[email protected]>
>>> ---
>>>   drivers/hwtracing/coresight/coresight-tpdm.c | 58
>>> ++++++++++++++++++++
>>>   drivers/hwtracing/coresight/coresight-tpdm.h | 23 ++++++++
>>>   2 files changed, 81 insertions(+)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c
>>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>>> index 6a4e2a35053d..70df888ac565 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>>> @@ -20,7 +20,28 @@
>>>   DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>>> +static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>>> +{
>>> +    u32 val;
>>> +
>>> +    /* Set the enable bit of DSB control register to 1 */
>>> +    val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
>>> +    val |= TPDM_DSB_CR_ENA;
>>> +    writel_relaxed(val, drvdata->base + TPDM_DSB_CR);
>>> +}
>>> +
>
>
>
>>>   /* TPDM enable operations */
>>> +static void _tpdm_enable(struct tpdm_drvdata *drvdata)
>
>
>>>   /* TPDM disable operations */
>>> +static void _tpdm_disable(struct tpdm_drvdata *drvdata)
>
> Missed this. The general convention is to use:
>
> __tpdm_disable()
> __tpdm_enable();
I will address your comments.
>
> So, please switch to the names above.
>
> Suzuki

2022-06-01 19:07:26

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v7 02/10] Coresight: Add coresight TPDM source driver

On 01/06/2022 10:21, Jinlong Mao wrote:
> Hi Suzuki,
>
> On 5/24/2022 3:00 PM, Jinlong Mao wrote:
>> Hi Suzuki,
>>
>> Thank you for the review.
>>
>> On 5/23/2022 4:57 PM, Suzuki K Poulose wrote:
>>> Hi
>>>
>>> On 09/05/2022 14:39, Mao Jinlong wrote:
>>>> Add driver to support Coresight device TPDM (Trace, Profiling and
>>>> Diagnostics Monitor). TPDM is a monitor to collect data from
>>>> different datasets. This change is to add probe/enable/disable
>>>> functions for tpdm source.
>>>>
>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>> Signed-off-by: Mao Jinlong <[email protected]>
>>>> ---
>>>>   drivers/hwtracing/coresight/Kconfig          |  13 ++
>>>>   drivers/hwtracing/coresight/Makefile         |   1 +
>>>>   drivers/hwtracing/coresight/coresight-core.c |   5 +-
>>>>   drivers/hwtracing/coresight/coresight-tpdm.c | 146
>>>> +++++++++++++++++++
>>>>   drivers/hwtracing/coresight/coresight-tpdm.h |  26 ++++
>>>>   include/linux/coresight.h                    |   1 +
>>>>   6 files changed, 191 insertions(+), 1 deletion(-)
>>>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c
>>>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h
>>>>
>
>>>> +/**
>>>> + * struct tpdm_drvdata - specifics associated to an TPDM component
>>>> + * @base:       memory mapped base address for this component.
>>>> + * @dev:        The device entity associated to this component.
>>>> + * @csdev:      component vitals needed by the framework.
>>>> + * @lock:       lock for the enable value.
>>>> + * @enable:     enable status of the component.
>>>> + */
>>>> +
>>>> +struct tpdm_drvdata {
>>>> +    void __iomem        *base;
>>>> +    struct device        *dev;
>>>> +    struct coresight_device    *csdev;
>>>> +    struct mutex        lock;
>>>
>>> Why mutex lock ? Couldn't this be a spinlock ?
>> 1. There is no irq for TPDM
>> 2. As there are 7 dataset types, there will be some FOR loop to configure
>> tpdm registers which may cause some time.

How long does it take to configure ? Is it too long enough to trigger
RCU stalls ? as long as we don't do any sleeping/blocking operations
we should be fine with a spinlock.

Suzuki

>>
> I think we can use mutex lock here. Do you have any more comments for
> this ?

>
> Thanks
> Jinlong Mao
>>>
>>>> +    bool            enable;
>>>> +};
>>>> +
>>>> +#endif  /* _CORESIGHT_CORESIGHT_TPDM_H */
>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>>> index 247147c11231..a9efac55029d 100644
>>>> --- a/include/linux/coresight.h
>>>> +++ b/include/linux/coresight.h
>>>> @@ -61,6 +61,7 @@ enum coresight_dev_subtype_source {
>>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
>>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
>>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
>>>> +    CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY,
>>>
>>> super minor nit: I find the choice of name a bit odd.
>>> We could simply make it something like :
>>>
>>>     CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS:
>>>
>>> Suzuki
>> I will check and update.
>>>
>>>>   };
>>>>     enum coresight_dev_subtype_helper {
>>>
>>> _______________________________________________
>>> CoreSight mailing list -- [email protected]
>>> To unsubscribe send an email to [email protected]
>> _______________________________________________
>> CoreSight mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]


2022-06-01 19:10:09

by Mao Jinlong

[permalink] [raw]
Subject: Re: [PATCH v7 02/10] Coresight: Add coresight TPDM source driver


On 6/1/2022 5:30 PM, Suzuki K Poulose wrote:
> On 01/06/2022 10:21, Jinlong Mao wrote:
>> Hi Suzuki,
>>
>> On 5/24/2022 3:00 PM, Jinlong Mao wrote:
>>> Hi Suzuki,
>>>
>>> Thank you for the review.
>>>
>>> On 5/23/2022 4:57 PM, Suzuki K Poulose wrote:
>>>> Hi
>>>>
>>>> On 09/05/2022 14:39, Mao Jinlong wrote:
>>>>> Add driver to support Coresight device TPDM (Trace, Profiling and
>>>>> Diagnostics Monitor). TPDM is a monitor to collect data from
>>>>> different datasets. This change is to add probe/enable/disable
>>>>> functions for tpdm source.
>>>>>
>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>> Signed-off-by: Mao Jinlong <[email protected]>
>>>>> ---
>>>>>   drivers/hwtracing/coresight/Kconfig          |  13 ++
>>>>>   drivers/hwtracing/coresight/Makefile         |   1 +
>>>>>   drivers/hwtracing/coresight/coresight-core.c |   5 +-
>>>>>   drivers/hwtracing/coresight/coresight-tpdm.c | 146
>>>>> +++++++++++++++++++
>>>>>   drivers/hwtracing/coresight/coresight-tpdm.h |  26 ++++
>>>>>   include/linux/coresight.h                    |   1 +
>>>>>   6 files changed, 191 insertions(+), 1 deletion(-)
>>>>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c
>>>>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h
>>>>>
>>
>>>>> +/**
>>>>> + * struct tpdm_drvdata - specifics associated to an TPDM component
>>>>> + * @base:       memory mapped base address for this component.
>>>>> + * @dev:        The device entity associated to this component.
>>>>> + * @csdev:      component vitals needed by the framework.
>>>>> + * @lock:       lock for the enable value.
>>>>> + * @enable:     enable status of the component.
>>>>> + */
>>>>> +
>>>>> +struct tpdm_drvdata {
>>>>> +    void __iomem        *base;
>>>>> +    struct device        *dev;
>>>>> +    struct coresight_device    *csdev;
>>>>> +    struct mutex        lock;
>>>>
>>>> Why mutex lock ? Couldn't this be a spinlock ?
>>> 1. There is no irq for TPDM
>>> 2. As there are 7 dataset types, there will be some FOR loop to
>>> configure
>>> tpdm registers which may cause some time.
>
> How long does it take to configure ? Is it too long enough to trigger
> RCU stalls ? as long as we don't do any sleeping/blocking operations
> we should be fine with a spinlock.
>
> Suzuki

Let me check on internal device and get back to you.

Thanks
Jinlong Mao
>
>>>
>> I think we can use mutex lock here. Do you have any more comments for
>> this ?
>
>>
>> Thanks
>> Jinlong Mao
>>>>
>>>>> +    bool            enable;
>>>>> +};
>>>>> +
>>>>> +#endif  /* _CORESIGHT_CORESIGHT_TPDM_H */
>>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>>>> index 247147c11231..a9efac55029d 100644
>>>>> --- a/include/linux/coresight.h
>>>>> +++ b/include/linux/coresight.h
>>>>> @@ -61,6 +61,7 @@ enum coresight_dev_subtype_source {
>>>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
>>>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
>>>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
>>>>> +    CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY,
>>>>
>>>> super minor nit: I find the choice of name a bit odd.
>>>> We could simply make it something like :
>>>>
>>>>     CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS:
>>>>
>>>> Suzuki
>>> I will check and update.
>>>>
>>>>>   };
>>>>>     enum coresight_dev_subtype_helper {
>>>>
>>>> _______________________________________________
>>>> CoreSight mailing list -- [email protected]
>>>> To unsubscribe send an email to [email protected]
>>> _______________________________________________
>>> CoreSight mailing list -- [email protected]
>>> To unsubscribe send an email to [email protected]
>
> _______________________________________________
> CoreSight mailing list -- [email protected]
> To unsubscribe send an email to [email protected]

2022-06-01 20:29:26

by Mao Jinlong

[permalink] [raw]
Subject: Re: [PATCH v7 02/10] Coresight: Add coresight TPDM source driver

Hi Suzuki,

On 5/24/2022 3:00 PM, Jinlong Mao wrote:
> Hi Suzuki,
>
> Thank you for the review.
>
> On 5/23/2022 4:57 PM, Suzuki K Poulose wrote:
>> Hi
>>
>> On 09/05/2022 14:39, Mao Jinlong wrote:
>>> Add driver to support Coresight device TPDM (Trace, Profiling and
>>> Diagnostics Monitor). TPDM is a monitor to collect data from
>>> different datasets. This change is to add probe/enable/disable
>>> functions for tpdm source.
>>>
>>> Signed-off-by: Tao Zhang <[email protected]>
>>> Signed-off-by: Mao Jinlong <[email protected]>
>>> ---
>>>   drivers/hwtracing/coresight/Kconfig          |  13 ++
>>>   drivers/hwtracing/coresight/Makefile         |   1 +
>>>   drivers/hwtracing/coresight/coresight-core.c |   5 +-
>>>   drivers/hwtracing/coresight/coresight-tpdm.c | 146
>>> +++++++++++++++++++
>>>   drivers/hwtracing/coresight/coresight-tpdm.h |  26 ++++
>>>   include/linux/coresight.h                    |   1 +
>>>   6 files changed, 191 insertions(+), 1 deletion(-)
>>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c
>>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h
>>>

>>> +/**
>>> + * struct tpdm_drvdata - specifics associated to an TPDM component
>>> + * @base:       memory mapped base address for this component.
>>> + * @dev:        The device entity associated to this component.
>>> + * @csdev:      component vitals needed by the framework.
>>> + * @lock:       lock for the enable value.
>>> + * @enable:     enable status of the component.
>>> + */
>>> +
>>> +struct tpdm_drvdata {
>>> +    void __iomem        *base;
>>> +    struct device        *dev;
>>> +    struct coresight_device    *csdev;
>>> +    struct mutex        lock;
>>
>> Why mutex lock ? Couldn't this be a spinlock ?
> 1. There is no irq for TPDM
> 2. As there are 7 dataset types, there will be some FOR loop to configure
> tpdm registers which may cause some time.
>
I think we can use mutex lock here. Do you have any more comments for this ?

Thanks
Jinlong Mao
>>
>>> +    bool            enable;
>>> +};
>>> +
>>> +#endif  /* _CORESIGHT_CORESIGHT_TPDM_H */
>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>> index 247147c11231..a9efac55029d 100644
>>> --- a/include/linux/coresight.h
>>> +++ b/include/linux/coresight.h
>>> @@ -61,6 +61,7 @@ enum coresight_dev_subtype_source {
>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
>>> +    CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY,
>>
>> super minor nit: I find the choice of name a bit odd.
>> We could simply make it something like :
>>
>>     CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS:
>>
>> Suzuki
> I will check and update.
>>
>>>   };
>>>     enum coresight_dev_subtype_helper {
>>
>> _______________________________________________
>> CoreSight mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]
> _______________________________________________
> CoreSight mailing list -- [email protected]
> To unsubscribe send an email to [email protected]

2022-06-02 03:16:29

by Mao Jinlong

[permalink] [raw]
Subject: Re: [PATCH v7 02/10] Coresight: Add coresight TPDM source driver


On 6/1/2022 5:56 PM, Jinlong Mao wrote:
>
> On 6/1/2022 5:30 PM, Suzuki K Poulose wrote:
>> On 01/06/2022 10:21, Jinlong Mao wrote:
>>> Hi Suzuki,
>>>
>>> On 5/24/2022 3:00 PM, Jinlong Mao wrote:
>>>> Hi Suzuki,
>>>>
>>>> Thank you for the review.
>>>>
>>>> On 5/23/2022 4:57 PM, Suzuki K Poulose wrote:
>>>>> Hi
>>>>>
>>>>> On 09/05/2022 14:39, Mao Jinlong wrote:
>>>>>> Add driver to support Coresight device TPDM (Trace, Profiling and
>>>>>> Diagnostics Monitor). TPDM is a monitor to collect data from
>>>>>> different datasets. This change is to add probe/enable/disable
>>>>>> functions for tpdm source.
>>>>>>
>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>> Signed-off-by: Mao Jinlong <[email protected]>
>>>>>> ---
>>>>>>   drivers/hwtracing/coresight/Kconfig          |  13 ++
>>>>>>   drivers/hwtracing/coresight/Makefile         |   1 +
>>>>>>   drivers/hwtracing/coresight/coresight-core.c |   5 +-
>>>>>>   drivers/hwtracing/coresight/coresight-tpdm.c | 146
>>>>>> +++++++++++++++++++
>>>>>>   drivers/hwtracing/coresight/coresight-tpdm.h |  26 ++++
>>>>>>   include/linux/coresight.h                    |   1 +
>>>>>>   6 files changed, 191 insertions(+), 1 deletion(-)
>>>>>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c
>>>>>>   create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h
>>>>>>
>>>
>>>>>> +/**
>>>>>> + * struct tpdm_drvdata - specifics associated to an TPDM component
>>>>>> + * @base:       memory mapped base address for this component.
>>>>>> + * @dev:        The device entity associated to this component.
>>>>>> + * @csdev:      component vitals needed by the framework.
>>>>>> + * @lock:       lock for the enable value.
>>>>>> + * @enable:     enable status of the component.
>>>>>> + */
>>>>>> +
>>>>>> +struct tpdm_drvdata {
>>>>>> +    void __iomem        *base;
>>>>>> +    struct device        *dev;
>>>>>> +    struct coresight_device    *csdev;
>>>>>> +    struct mutex        lock;
>>>>>
>>>>> Why mutex lock ? Couldn't this be a spinlock ?
>>>> 1. There is no irq for TPDM
>>>> 2. As there are 7 dataset types, there will be some FOR loop to
>>>> configure
>>>> tpdm registers which may cause some time.
>>
>> How long does it take to configure ? Is it too long enough to trigger
>> RCU stalls ? as long as we don't do any sleeping/blocking operations
>> we should be fine with a spinlock.
>>
>> Suzuki
>
> Let me check on internal device and get back to you.
>
> Thanks
> Jinlong Mao

The time of configuring the registers doesn't reach RCU stall timeout value.

I will use spin_lock for both tpdm and tpda.

Thanks
Jinlong Mao
>>
>>>>
>>> I think we can use mutex lock here. Do you have any more comments
>>> for this ?
>>
>>>
>>> Thanks
>>> Jinlong Mao
>>>>>
>>>>>> +    bool            enable;
>>>>>> +};
>>>>>> +
>>>>>> +#endif  /* _CORESIGHT_CORESIGHT_TPDM_H */
>>>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>>>>> index 247147c11231..a9efac55029d 100644
>>>>>> --- a/include/linux/coresight.h
>>>>>> +++ b/include/linux/coresight.h
>>>>>> @@ -61,6 +61,7 @@ enum coresight_dev_subtype_source {
>>>>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
>>>>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
>>>>>>       CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
>>>>>> +    CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY,
>>>>>
>>>>> super minor nit: I find the choice of name a bit odd.
>>>>> We could simply make it something like :
>>>>>
>>>>>     CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS:
>>>>>
>>>>> Suzuki
>>>> I will check and update.
>>>>>
>>>>>>   };
>>>>>>     enum coresight_dev_subtype_helper {
>>>>>
>>>>> _______________________________________________
>>>>> CoreSight mailing list -- [email protected]
>>>>> To unsubscribe send an email to [email protected]
>>>> _______________________________________________
>>>> CoreSight mailing list -- [email protected]
>>>> To unsubscribe send an email to [email protected]
>>
>> _______________________________________________
>> CoreSight mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]
> _______________________________________________
> CoreSight mailing list -- [email protected]
> To unsubscribe send an email to [email protected]