[PATCH v4 0/3] Add support to configure Coresight Dummy subunit

[PATCH v4 0/3] Add support to configure Coresight Dummy subunit

[Hao Zhang]

Introduction of Coresight Dummy subunit
The Coresight Dummy subunit is for Coresight Dummy component, there are
some specific Coresight devices that HLOS don't have permission to access.
Such as some TPDMs, they would be configured in NON-HLOS side, but it's
necessary to build Coresight path for it to debug. So there need driver
to register dummy devices as Coresight devices.

Commit link:
https://git.codelinaro.org/clo/linux-kernel/coresight/-/tree/coresight-dummy-v4

Changes in V4:
1. Remove traceid allocation in dummy_probe function since it is
currently not in use, will upstream it as the part of ATID filtering
in the further.  -- Suzuki K Poulose
2. Remove 'oneOf' as there is only one entry. -- Rob Herring

Changes in V3:
1. Use API "dev_dbg" to replace "dev_info". -- Suzuki K Poulose
2. Drop "qcom" property and take it as a dummy framework.
-- Suzuki K Poulose
3. Add new sub-type "CORESIGHT_DEV_SUBTYPE_SINK_DUMMY" to support
coresight dummy module -- Mike Leach
4. Use compatibles "arm,coresight-dummy-source" and
"arm,coresight-dummy-sink" to replace property "qcom,dummy-source" and
"qcom,dummy-sink". -- Mike Leach
5. Define source_devs and sink_devs DEVLIST to replace dummy_devs, make
it clear at the first level. -- Mike Leach
6. Modify subject of YAML patch, drop "YAML schema". -- Krzysztof Kozlowski
7. Drop some redundant items and correct syntax errors in yaml file.
-- Krzysztof Kozlowski/Rob Herring
8. Correct required property of yaml file, constrain out ports to
dummy-source and in ports to dummy-sink. -- Mike Leach
9. Drop "Sysfs files and directories" contents of coresight-dummy.rst.
-- Suzuki K Poulose/Greg Kroah-Hartman
10.Correct syntax errors of coresight-dummy.rst. -- Bagas Sanjaya

Changes in V2:
1. Declare dummy_init and dummy_exit as static to fix missing-prototypes
warnings. -- kernel test robot
2. Fix the errors of coresight-dummy yaml file. -- Rob Herring

Hao Zhang (3):
  Coresight: Add coresight dummy driver
  dt-bindings: arm: Add Coresight Dummy Trace
  Documentation: trace: Add documentation for Coresight Dummy Trace

 .../bindings/arm/arm,coresight-dummy.yaml     | 102 +++++++++++
 .../trace/coresight/coresight-dummy.rst       |  34 ++++
 drivers/hwtracing/coresight/Kconfig           |  11 ++
 drivers/hwtracing/coresight/Makefile          |   1 +
 drivers/hwtracing/coresight/coresight-dummy.c | 171 ++++++++++++++++++
 include/linux/coresight.h                     |   1 +
 6 files changed, 320 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
 create mode 100644 Documentation/trace/coresight/coresight-dummy.rst
 create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c

-- 
2.17.1

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

[Hao Zhang]

Some Coresight devices that kernel don't have permission to access or
configure. So there need driver to register dummy devices as Coresight
devices. It may also be used to define components that may not have
any programming interfaces (e.g, static links), so that paths can be
established in the driver. Provide Coresight API for dummy device
operations, such as enabling and disabling dummy devices. Build the
Coresight path for dummy sink or dummy source for debugging.

Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
---
 drivers/hwtracing/coresight/Kconfig           |  11 ++
 drivers/hwtracing/coresight/Makefile          |   1 +
 drivers/hwtracing/coresight/coresight-dummy.c | 171 ++++++++++++++++++
 include/linux/coresight.h                     |   1 +
 4 files changed, 184 insertions(+)
 create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c

diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index 2b5bbfffbc4f..06f0a7594169 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -236,4 +236,15 @@ config CORESIGHT_TPDA
 
 	  To compile this driver as a module, choose M here: the module will be
 	  called coresight-tpda.
+
+config CORESIGHT_DUMMY
+	tristate "Dummy driver support"
+	help
+	  Enables support for dummy driver. Dummy driver can be used for
+	  CoreSight sources/sinks that are owned and configured by some
+	  other subsystem and use Linux drivers to configure rest of trace
+	  path.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called coresight-dummy.
 endif
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index 33bcc3f7b8ae..995d3b2c76df 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o
 coresight-cti-y := coresight-cti-core.o	coresight-cti-platform.o \
 		   coresight-cti-sysfs.o
 obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
+obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c
new file mode 100644
index 000000000000..ee9881ff4754
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-dummy.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/coresight.h>
+#include <linux/of.h>
+#include <linux/pm_runtime.h>
+
+#include "coresight-priv.h"
+
+struct dummy_drvdata {
+	struct device			*dev;
+	struct coresight_device		*csdev;
+};
+
+DEFINE_CORESIGHT_DEVLIST(source_devs, "dummy_source");
+DEFINE_CORESIGHT_DEVLIST(sink_devs, "dummy_sink");
+
+static int dummy_source_enable(struct coresight_device *csdev,
+			       struct perf_event *event, u32 mode)
+{
+	struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	dev_dbg(drvdata->dev, "Dummy source enabled\n");
+
+	return 0;
+}
+
+static void dummy_source_disable(struct coresight_device *csdev,
+				 struct perf_event *event)
+{
+	struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	dev_dbg(drvdata->dev, "Dummy source disabled\n");
+}
+
+static int dummy_sink_enable(struct coresight_device *csdev, u32 mode,
+				void *data)
+{
+	struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	dev_dbg(drvdata->dev, "Dummy sink enabled\n");
+
+	return 0;
+}
+
+static int dummy_sink_disable(struct coresight_device *csdev)
+{
+	struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	dev_dbg(drvdata->dev, "Dummy sink disabled\n");
+
+	return 0;
+}
+
+static const struct coresight_ops_source dummy_source_ops = {
+	.enable	= dummy_source_enable,
+	.disable = dummy_source_disable,
+};
+
+static const struct coresight_ops dummy_source_cs_ops = {
+	.source_ops = &dummy_source_ops,
+};
+
+static const struct coresight_ops_sink dummy_sink_ops = {
+	.enable	= dummy_sink_enable,
+	.disable = dummy_sink_disable,
+};
+
+static const struct coresight_ops dummy_sink_cs_ops = {
+	.sink_ops = &dummy_sink_ops,
+};
+
+static int dummy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct coresight_platform_data *pdata;
+	struct dummy_drvdata *drvdata;
+	struct coresight_desc desc = { 0 };
+
+	if (of_device_is_compatible(node, "arm,coresight-dummy-source")) {
+
+		desc.name = coresight_alloc_device_name(&source_devs, dev);
+		if (!desc.name)
+			return -ENOMEM;
+
+		desc.type = CORESIGHT_DEV_TYPE_SOURCE;
+		desc.subtype.source_subtype =
+					CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS;
+		desc.ops = &dummy_source_cs_ops;
+	} else if (of_device_is_compatible(node, "arm,coresight-dummy-sink")) {
+		desc.name = coresight_alloc_device_name(&sink_devs, dev);
+		if (!desc.name)
+			return -ENOMEM;
+
+		desc.type = CORESIGHT_DEV_TYPE_SINK;
+		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_DUMMY;
+		desc.ops = &dummy_sink_cs_ops;
+	} else {
+		dev_err(dev, "Device type not set\n");
+		return -EINVAL;
+	}
+
+	pdata = coresight_get_platform_data(dev);
+	if (IS_ERR(pdata))
+		return PTR_ERR(pdata);
+	pdev->dev.platform_data = pdata;
+
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	drvdata->dev = &pdev->dev;
+	platform_set_drvdata(pdev, drvdata);
+
+	desc.pdata = pdev->dev.platform_data;
+	desc.dev = &pdev->dev;
+	drvdata->csdev = coresight_register(&desc);
+	if (IS_ERR(drvdata->csdev))
+		return PTR_ERR(drvdata->csdev);
+
+	pm_runtime_enable(dev);
+	dev_dbg(dev, "Dummy device initialized\n");
+
+	return 0;
+}
+
+static int dummy_remove(struct platform_device *pdev)
+{
+	struct dummy_drvdata *drvdata = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+
+	pm_runtime_disable(dev);
+	coresight_unregister(drvdata->csdev);
+	return 0;
+}
+
+static const struct of_device_id dummy_match[] = {
+	{.compatible = "arm,coresight-dummy-source"},
+	{.compatible = "arm,coresight-dummy-sink"},
+	{},
+};
+
+static struct platform_driver dummy_driver = {
+	.probe	= dummy_probe,
+	.remove	= dummy_remove,
+	.driver	= {
+		.name   = "coresight-dummy",
+		.of_match_table = dummy_match,
+	},
+};
+
+static int __init dummy_init(void)
+{
+	return platform_driver_register(&dummy_driver);
+}
+module_init(dummy_init);
+
+static void __exit dummy_exit(void)
+{
+	platform_driver_unregister(&dummy_driver);
+}
+module_exit(dummy_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("CoreSight dummy driver");
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index f19a47b9bb5a..6db4b49751cf 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -45,6 +45,7 @@ enum coresight_dev_type {
 };
 
 enum coresight_dev_subtype_sink {
+	CORESIGHT_DEV_SUBTYPE_SINK_DUMMY,
 	CORESIGHT_DEV_SUBTYPE_SINK_PORT,
 	CORESIGHT_DEV_SUBTYPE_SINK_BUFFER,
 	CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM,
-- 
2.17.1

[PATCH v4 2/3] dt-bindings: arm: Add Coresight Dummy Trace

[Hao Zhang]

Add new coresight-dummy.yaml file describing the bindings required
to define coresight dummy trace in the device trees.

Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
---
 .../bindings/arm/arm,coresight-dummy.yaml     | 102 ++++++++++++++++++
 1 file changed, 102 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml

diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
new file mode 100644
index 000000000000..126518863eea
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
@@ -0,0 +1,102 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/arm,coresight-dummy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM Coresight Dummy component
+
+description: |
+  Coresight Dummy Trace Module is for the specific devices that kernel
+  don't have permission to access or configure, e.g., CoreSight TPDMs
+  on Qualcomm platforms. So there need driver to register dummy devices
+  as Coresight devices. It may also be used to define components that
+  may not have any programming interfaces (e.g, static links), so that
+  paths can be established in the driver. Provide Coresight API for
+  dummy device operations, such as enabling and disabling dummy devices.
+  Build the Coresight path for dummy sink or dummy source for debugging.
+
+  The primary use case of the coresight dummy is to build path in kernel
+  side for dummy sink and dummy source.
+
+maintainers:
+  - Mao Jinlong <quic_jinlmao@quicinc.com>
+  - Tao Zhang <quic_taozha@quicinc.com>
+  - Hao Zhang <quic_hazha@quicinc.com>
+  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - arm,coresight-dummy-sink
+          - arm,coresight-dummy-source
+
+  out-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port:
+        description: Output connection from the source to Coresight
+          Trace bus.
+        $ref: /schemas/graph.yaml#/properties/port
+
+  in-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port:
+        description: Input connection from the Coresight Trace bus to
+          dummy sink, such as Embedded USB debugger(EUD).
+        $ref: /schemas/graph.yaml#/properties/port
+
+required:
+  - compatible
+
+if:
+  # If the compatible contains the below value
+  properties:
+    compatible:
+      contains:
+        const: arm,coresight-dummy-sink
+
+then:
+  required:
+    - in-ports
+
+else:
+  required:
+    - out-ports
+
+additionalProperties: false
+
+examples:
+  # Minimum dummy sink definition. Dummy sink connect to coresight replicator.
+  - |
+    sink {
+      compatible = "arm,coresight-dummy-sink";
+
+      in-ports {
+        port {
+          eud_in_replicator_swao: endpoint {
+            remote-endpoint = <&replicator_swao_out_eud>;
+          };
+        };
+      };
+    };
+
+  # Minimum dummy source definition. Dummy source connect to coresight funnel.
+  - |
+    source {
+      compatible = "arm,coresight-dummy-source";
+
+      out-ports {
+        port {
+          dummy_riscv_out_funnel_swao: endpoint {
+            remote-endpoint = <&funnel_swao_in_dummy_riscv>;
+          };
+        };
+      };
+    };
+
+...
-- 
2.17.1

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

[Hao Zhang]

Add documentation for Coresight Dummy Trace under trace/coresight.

Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
---
 .../trace/coresight/coresight-dummy.rst       | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/trace/coresight/coresight-dummy.rst

diff --git a/Documentation/trace/coresight/coresight-dummy.rst b/Documentation/trace/coresight/coresight-dummy.rst
new file mode 100644
index 000000000000..7cb59f080c88
--- /dev/null
+++ b/Documentation/trace/coresight/coresight-dummy.rst
@@ -0,0 +1,34 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============================
+Coresight Dummy Trace Module
+=============================
+
+    :Author:   Hao Zhang <quic_hazha@quicinc.com>
+    :Date:     May 2023
+
+Introduction
+---------------------------
+
+Coresight Dummy Trace Module is for the specific devices that kernel
+don't have permission to access or configure, e.g., CoreSight TPDMs
+on Qualcomm platforms. So there need driver to register dummy devices
+as Coresight devices. It may also be used to define components that
+may not have any programming interfaces (e.g, static links), so that
+paths can be established in the driver. Provide Coresight API for
+dummy device operations, such as enabling and disabling dummy devices.
+Build the Coresight path for dummy sink or dummy source for debugging.
+
+Config details
+---------------------------
+
+There are two types of nodes, dummy sink and dummy source. The nodes
+should be observed at the below coresight path::
+
+    ``/sys/bus/coresight/devices``.
+
+e.g.::
+
+    / $ ls -l /sys/bus/coresight/devices | grep dummy
+    dummy_sink0 -> ../../../devices/platform/soc@0/soc@0:sink/dummy_sink0
+    dummy_source0 -> ../../../devices/platform/soc@0/soc@0:source/dummy_source0
-- 
2.17.1

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

[Suzuki K Poulose]

On 05/05/2023 10:24, Hao Zhang wrote:
> Some Coresight devices that kernel don't have permission to access or
> configure. So there need driver to register dummy devices as Coresight
> devices. It may also be used to define components that may not have
> any programming interfaces (e.g, static links), so that paths can be
> established in the driver. Provide Coresight API for dummy device
> operations, such as enabling and disabling dummy devices. Build the
> Coresight path for dummy sink or dummy source for debugging.
> 
> Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
> ---
>   drivers/hwtracing/coresight/Kconfig           |  11 ++
>   drivers/hwtracing/coresight/Makefile          |   1 +
>   drivers/hwtracing/coresight/coresight-dummy.c | 171 ++++++++++++++++++
>   include/linux/coresight.h                     |   1 +
>   4 files changed, 184 insertions(+)
>   create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c
> 
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 2b5bbfffbc4f..06f0a7594169 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -236,4 +236,15 @@ config CORESIGHT_TPDA
>   
>   	  To compile this driver as a module, choose M here: the module will be
>   	  called coresight-tpda.
> +
> +config CORESIGHT_DUMMY
> +	tristate "Dummy driver support"
> +	help
> +	  Enables support for dummy driver. Dummy driver can be used for
> +	  CoreSight sources/sinks that are owned and configured by some
> +	  other subsystem and use Linux drivers to configure rest of trace
> +	  path > +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called coresight-dummy.
>   endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 33bcc3f7b8ae..995d3b2c76df 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -30,3 +30,4 @@ obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o
>   coresight-cti-y := coresight-cti-core.o	coresight-cti-platform.o \
>   		   coresight-cti-sysfs.o
>   obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
> +obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c
> new file mode 100644
> index 000000000000..ee9881ff4754
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-dummy.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/coresight.h>
> +#include <linux/of.h>
> +#include <linux/pm_runtime.h>

Please follow the alphabetical order for the header files ^

> +
> +#include "coresight-priv.h"
> +
> +struct dummy_drvdata {
> +	struct device			*dev;

nit: We don't need this really. And that completely removes the need for
drvdata too. See below.

> +	struct coresight_device		*csdev;
> +};
> +
> +DEFINE_CORESIGHT_DEVLIST(source_devs, "dummy_source");
> +DEFINE_CORESIGHT_DEVLIST(sink_devs, "dummy_sink");
> +
> +static int dummy_source_enable(struct coresight_device *csdev,
> +			       struct perf_event *event, u32 mode)
> +{
> +	struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	dev_dbg(drvdata->dev, "Dummy source enabled\n");

	dev_dbg(csdev->dev.parent, ..");

Similarly for all instances below.

> +
> +	return 0;
> +}
> +
> +static void dummy_source_disable(struct coresight_device *csdev,
> +				 struct perf_event *event)
> +{
> +	struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	dev_dbg(drvdata->dev, "Dummy source disabled\n");
> +}
> +
> +static int dummy_sink_enable(struct coresight_device *csdev, u32 mode,
> +				void *data)
> +{
> +	struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	dev_dbg(drvdata->dev, "Dummy sink enabled\n");
> +
> +	return 0;
> +}
> +
> +static int dummy_sink_disable(struct coresight_device *csdev)
> +{
> +	struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	dev_dbg(drvdata->dev, "Dummy sink disabled\n");
> +
> +	return 0;
> +}
> +
> +static const struct coresight_ops_source dummy_source_ops = {
> +	.enable	= dummy_source_enable,
> +	.disable = dummy_source_disable,
> +};
> +
> +static const struct coresight_ops dummy_source_cs_ops = {
> +	.source_ops = &dummy_source_ops,
> +};
> +
> +static const struct coresight_ops_sink dummy_sink_ops = {
> +	.enable	= dummy_sink_enable,
> +	.disable = dummy_sink_disable,
> +};
> +
> +static const struct coresight_ops dummy_sink_cs_ops = {
> +	.sink_ops = &dummy_sink_ops,
> +};
> +
> +static int dummy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct coresight_platform_data *pdata;
> +	struct dummy_drvdata *drvdata;
> +	struct coresight_desc desc = { 0 };
> +
> +	if (of_device_is_compatible(node, "arm,coresight-dummy-source")) {
> +
> +		desc.name = coresight_alloc_device_name(&source_devs, dev);
> +		if (!desc.name)
> +			return -ENOMEM;
> +
> +		desc.type = CORESIGHT_DEV_TYPE_SOURCE;
> +		desc.subtype.source_subtype =
> +					CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS;
> +		desc.ops = &dummy_source_cs_ops;
> +	} else if (of_device_is_compatible(node, "arm,coresight-dummy-sink")) {
> +		desc.name = coresight_alloc_device_name(&sink_devs, dev);
> +		if (!desc.name)
> +			return -ENOMEM;
> +
> +		desc.type = CORESIGHT_DEV_TYPE_SINK;
> +		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_DUMMY;
> +		desc.ops = &dummy_sink_cs_ops;
> +	} else {
> +		dev_err(dev, "Device type not set\n");
> +		return -EINVAL;
> +	}
> +
> +	pdata = coresight_get_platform_data(dev);
> +	if (IS_ERR(pdata))
> +		return PTR_ERR(pdata);
> +	pdev->dev.platform_data = pdata;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM; > +
> +	drvdata->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, drvdata);

As above, you may remove the drvdata entirely.

Otherwise looks good to me

Suzuki

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

[Suzuki K Poulose]

On 05/05/2023 10:24, Hao Zhang wrote:
> Add new coresight-dummy.yaml file describing the bindings required
> to define coresight dummy trace in the device trees.
> 
> Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
> ---
>   .../bindings/arm/arm,coresight-dummy.yaml     | 102 ++++++++++++++++++
>   1 file changed, 102 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
> new file mode 100644
> index 000000000000..126518863eea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/arm,coresight-dummy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM Coresight Dummy component
> +
> +description: |
> +  Coresight Dummy Trace Module is for the specific devices that kernel
> +  don't have permission to access or configure, e.g., CoreSight TPDMs
> +  on Qualcomm platforms. So there need driver to register dummy devices
> +  as Coresight devices. It may also be used to define components that
> +  may not have any programming interfaces (e.g, static links), so that
> +  paths can be established in the driver. Provide Coresight API for
> +  dummy device operations, such as enabling and disabling dummy devices.
> +  Build the Coresight path for dummy sink or dummy source for debugging. > +
> +  The primary use case of the coresight dummy is to build path in kernel
> +  side for dummy sink and dummy source.
> +
> +maintainers:
> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> +  - Tao Zhang <quic_taozha@quicinc.com>
> +  - Hao Zhang <quic_hazha@quicinc.com>
> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>

Given this is a generic "CoreSight" component, I would prefer to have 
the CoreSight subsystem maintainers listed here (too). I don't mind
the entries above, but would like to make sure that the subsystem
people are aware of the changes happening here. Please use:

Mike Leach <mike.leach@linaro.org>
Suzuki K Poulose <suzuki.poulose@arm.com>
Leo Yan <leo.yan@linaro.org>

With the above:

Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>

> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - arm,coresight-dummy-sink
> +          - arm,coresight-dummy-source
> +
> +  out-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port:
> +        description: Output connection from the source to Coresight
> +          Trace bus.
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +  in-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port:
> +        description: Input connection from the Coresight Trace bus to
> +          dummy sink, such as Embedded USB debugger(EUD).
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> +  - compatible
> +
> +if:
> +  # If the compatible contains the below value
> +  properties:
> +    compatible:
> +      contains:
> +        const: arm,coresight-dummy-sink
> +
> +then:
> +  required:
> +    - in-ports
> +
> +else:
> +  required:
> +    - out-ports
> +
> +additionalProperties: false
> +
> +examples:
> +  # Minimum dummy sink definition. Dummy sink connect to coresight replicator.
> +  - |
> +    sink {
> +      compatible = "arm,coresight-dummy-sink";
> +
> +      in-ports {
> +        port {
> +          eud_in_replicator_swao: endpoint {
> +            remote-endpoint = <&replicator_swao_out_eud>;
> +          };
> +        };
> +      };
> +    };
> +
> +  # Minimum dummy source definition. Dummy source connect to coresight funnel.
> +  - |
> +    source {
> +      compatible = "arm,coresight-dummy-source";
> +
> +      out-ports {
> +        port {
> +          dummy_riscv_out_funnel_swao: endpoint {
> +            remote-endpoint = <&funnel_swao_in_dummy_riscv>;
> +          };
> +        };
> +      };
> +    };
> +
> +...

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

[Leo Yan]

On Fri, May 05, 2023 at 11:54:03AM +0100, Suzuki Kuruppassery Poulose wrote:

[...]

> > +title: ARM Coresight Dummy component
> > +
> > +description: |
> > +  Coresight Dummy Trace Module is for the specific devices that kernel
> > +  don't have permission to access or configure, e.g., CoreSight TPDMs
> > +  on Qualcomm platforms. So there need driver to register dummy devices
> > +  as Coresight devices. It may also be used to define components that
> > +  may not have any programming interfaces (e.g, static links), so that
> > +  paths can be established in the driver. Provide Coresight API for
> > +  dummy device operations, such as enabling and disabling dummy devices.
> > +  Build the Coresight path for dummy sink or dummy source for debugging. > +
> > +  The primary use case of the coresight dummy is to build path in kernel
> > +  side for dummy sink and dummy source.
> > +
> > +maintainers:
> > +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> > +  - Tao Zhang <quic_taozha@quicinc.com>
> > +  - Hao Zhang <quic_hazha@quicinc.com>
> > +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> 
> Given this is a generic "CoreSight" component, I would prefer to have the
> CoreSight subsystem maintainers listed here (too). I don't mind
> the entries above, but would like to make sure that the subsystem
> people are aware of the changes happening here. Please use:
> 
> Mike Leach <mike.leach@linaro.org>
> Suzuki K Poulose <suzuki.poulose@arm.com>
> Leo Yan <leo.yan@linaro.org>

Given I am spending little time on CoreSight reviewing, I'd like to
use James Clark's email address to replace my own; I believe this
would benefit long term maintenance.

  James Clark <james.clark@arm.com>

Thanks!

> With the above:
> 
> Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - arm,coresight-dummy-sink
> > +          - arm,coresight-dummy-source
> > +
> > +  out-ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port:
> > +        description: Output connection from the source to Coresight
> > +          Trace bus.
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +
> > +  in-ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port:
> > +        description: Input connection from the Coresight Trace bus to
> > +          dummy sink, such as Embedded USB debugger(EUD).
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +
> > +required:
> > +  - compatible
> > +
> > +if:
> > +  # If the compatible contains the below value
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        const: arm,coresight-dummy-sink
> > +
> > +then:
> > +  required:
> > +    - in-ports
> > +
> > +else:
> > +  required:
> > +    - out-ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  # Minimum dummy sink definition. Dummy sink connect to coresight replicator.
> > +  - |
> > +    sink {
> > +      compatible = "arm,coresight-dummy-sink";
> > +
> > +      in-ports {
> > +        port {
> > +          eud_in_replicator_swao: endpoint {
> > +            remote-endpoint = <&replicator_swao_out_eud>;
> > +          };
> > +        };
> > +      };
> > +    };
> > +
> > +  # Minimum dummy source definition. Dummy source connect to coresight funnel.
> > +  - |
> > +    source {
> > +      compatible = "arm,coresight-dummy-source";
> > +
> > +      out-ports {
> > +        port {
> > +          dummy_riscv_out_funnel_swao: endpoint {
> > +            remote-endpoint = <&funnel_swao_in_dummy_riscv>;
> > +          };
> > +        };
> > +      };
> > +    };
> > +
> > +...
> 

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

[Krzysztof Kozlowski]

On 05/05/2023 11:24, Hao Zhang wrote:
> Add new coresight-dummy.yaml file describing the bindings required
> to define coresight dummy trace in the device trees.
> 
> Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
> ---
>  .../bindings/arm/arm,coresight-dummy.yaml     | 102 ++++++++++++++++++
>  1 file changed, 102 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
> new file mode 100644
> index 000000000000..126518863eea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/arm,coresight-dummy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM Coresight Dummy component
> +
> +description: |
> +  Coresight Dummy Trace Module is for the specific devices that kernel
> +  don't have permission to access or configure, e.g., CoreSight TPDMs
> +  on Qualcomm platforms. So there need driver to register dummy devices
> +  as Coresight devices. It may also be used to define components that
> +  may not have any programming interfaces (e.g, static links), so that
> +  paths can be established in the driver. Provide Coresight API for
> +  dummy device operations, such as enabling and disabling dummy devices.
> +  Build the Coresight path for dummy sink or dummy source for debugging.
> +
> +  The primary use case of the coresight dummy is to build path in kernel
> +  side for dummy sink and dummy source.
> +
> +maintainers:
> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> +  - Tao Zhang <quic_taozha@quicinc.com>
> +  - Hao Zhang <quic_hazha@quicinc.com>
> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> +
> +properties:
> +  compatible:
> +    items:

You were asked to drop oneOf, not to replace with items. Drop items.
Drop oneOf. It's just enum.

> +      - enum:
> +          - arm,coresight-dummy-sink
> +          - arm,coresight-dummy-source
> +
> +  out-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port:
> +        description: Output connection from the source to Coresight
> +          Trace bus.
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +  in-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port:
> +        description: Input connection from the Coresight Trace bus to
> +          dummy sink, such as Embedded USB debugger(EUD).
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> +  - compatible
> +
> +if:
> +  # If the compatible contains the below value
> +  properties:
> +    compatible:
> +      contains:
> +        const: arm,coresight-dummy-sink
> +
> +then:
> +  required:
> +    - in-ports
> +
> +else:
> +  required:
> +    - out-ports

No improvements. Implement Rob's comments.

Best regards,
Krzysztof

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

[Hao Zhang]

Hi Suzuki,

On 5/5/2023 5:57 PM, Suzuki K Poulose wrote:
> On 05/05/2023 10:24, Hao Zhang wrote:
>> Some Coresight devices that kernel don't have permission to access or
>> configure. So there need driver to register dummy devices as Coresight
>> devices. It may also be used to define components that may not have
>> any programming interfaces (e.g, static links), so that paths can be
>> established in the driver. Provide Coresight API for dummy device
>> operations, such as enabling and disabling dummy devices. Build the
>> Coresight path for dummy sink or dummy source for debugging.
>>
>> Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
>> ---
>>   drivers/hwtracing/coresight/Kconfig           |  11 ++
>>   drivers/hwtracing/coresight/Makefile          |   1 +
>>   drivers/hwtracing/coresight/coresight-dummy.c | 171 ++++++++++++++++++
>>   include/linux/coresight.h                     |   1 +
>>   4 files changed, 184 insertions(+)
>>   create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c
>>
>> diff --git a/drivers/hwtracing/coresight/Kconfig 
>> b/drivers/hwtracing/coresight/Kconfig
>> index 2b5bbfffbc4f..06f0a7594169 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -236,4 +236,15 @@ config CORESIGHT_TPDA
>>         To compile this driver as a module, choose M here: the module 
>> will be
>>         called coresight-tpda.
>> +
>> +config CORESIGHT_DUMMY
>> +    tristate "Dummy driver support"
>> +    help
>> +      Enables support for dummy driver. Dummy driver can be used for
>> +      CoreSight sources/sinks that are owned and configured by some
>> +      other subsystem and use Linux drivers to configure rest of trace
>> +      path > +
>> +      To compile this driver as a module, choose M here: the module 
>> will be
>> +      called coresight-dummy.
>>   endif
>> diff --git a/drivers/hwtracing/coresight/Makefile 
>> b/drivers/hwtracing/coresight/Makefile
>> index 33bcc3f7b8ae..995d3b2c76df 100644
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>> @@ -30,3 +30,4 @@ obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o
>>   coresight-cti-y := coresight-cti-core.o    coresight-cti-platform.o \
>>              coresight-cti-sysfs.o
>>   obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
>> +obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
>> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c 
>> b/drivers/hwtracing/coresight/coresight-dummy.c
>> new file mode 100644
>> index 000000000000..ee9881ff4754
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-dummy.c
>> @@ -0,0 +1,171 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights 
>> reserved.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/coresight.h>
>> +#include <linux/of.h>
>> +#include <linux/pm_runtime.h>
> 
> Please follow the alphabetical order for the header files ^
> 
Sure, I will update this in the next patch series.

>> +
>> +#include "coresight-priv.h"
>> +
>> +struct dummy_drvdata {
>> +    struct device            *dev;
> 
> nit: We don't need this really. And that completely removes the need for
> drvdata too. See below.
> 
>> +    struct coresight_device        *csdev;
>> +};
>> +
>> +DEFINE_CORESIGHT_DEVLIST(source_devs, "dummy_source");
>> +DEFINE_CORESIGHT_DEVLIST(sink_devs, "dummy_sink");
>> +
>> +static int dummy_source_enable(struct coresight_device *csdev,
>> +                   struct perf_event *event, u32 mode)
>> +{
>> +    struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    dev_dbg(drvdata->dev, "Dummy source enabled\n");
> 
>      dev_dbg(csdev->dev.parent, ..");
> 
> Similarly for all instances below.
> 
>> +
>> +    return 0;
>> +}
>> +
>> +static void dummy_source_disable(struct coresight_device *csdev,
>> +                 struct perf_event *event)
>> +{
>> +    struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    dev_dbg(drvdata->dev, "Dummy source disabled\n");
>> +}
>> +
>> +static int dummy_sink_enable(struct coresight_device *csdev, u32 mode,
>> +                void *data)
>> +{
>> +    struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    dev_dbg(drvdata->dev, "Dummy sink enabled\n");
>> +
>> +    return 0;
>> +}
>> +
>> +static int dummy_sink_disable(struct coresight_device *csdev)
>> +{
>> +    struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    dev_dbg(drvdata->dev, "Dummy sink disabled\n");
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct coresight_ops_source dummy_source_ops = {
>> +    .enable    = dummy_source_enable,
>> +    .disable = dummy_source_disable,
>> +};
>> +
>> +static const struct coresight_ops dummy_source_cs_ops = {
>> +    .source_ops = &dummy_source_ops,
>> +};
>> +
>> +static const struct coresight_ops_sink dummy_sink_ops = {
>> +    .enable    = dummy_sink_enable,
>> +    .disable = dummy_sink_disable,
>> +};
>> +
>> +static const struct coresight_ops dummy_sink_cs_ops = {
>> +    .sink_ops = &dummy_sink_ops,
>> +};
>> +
>> +static int dummy_probe(struct platform_device *pdev)
>> +{
>> +    struct device *dev = &pdev->dev;
>> +    struct device_node *node = dev->of_node;
>> +    struct coresight_platform_data *pdata;
>> +    struct dummy_drvdata *drvdata;
>> +    struct coresight_desc desc = { 0 };
>> +
>> +    if (of_device_is_compatible(node, "arm,coresight-dummy-source")) {
>> +
>> +        desc.name = coresight_alloc_device_name(&source_devs, dev);
>> +        if (!desc.name)
>> +            return -ENOMEM;
>> +
>> +        desc.type = CORESIGHT_DEV_TYPE_SOURCE;
>> +        desc.subtype.source_subtype =
>> +                    CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS;
>> +        desc.ops = &dummy_source_cs_ops;
>> +    } else if (of_device_is_compatible(node, 
>> "arm,coresight-dummy-sink")) {
>> +        desc.name = coresight_alloc_device_name(&sink_devs, dev);
>> +        if (!desc.name)
>> +            return -ENOMEM;
>> +
>> +        desc.type = CORESIGHT_DEV_TYPE_SINK;
>> +        desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_DUMMY;
>> +        desc.ops = &dummy_sink_cs_ops;
>> +    } else {
>> +        dev_err(dev, "Device type not set\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    pdata = coresight_get_platform_data(dev);
>> +    if (IS_ERR(pdata))
>> +        return PTR_ERR(pdata);
>> +    pdev->dev.platform_data = pdata;
>> +
>> +    drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> +    if (!drvdata)
>> +        return -ENOMEM; > +
>> +    drvdata->dev = &pdev->dev;
>> +    platform_set_drvdata(pdev, drvdata);
> 
> As above, you may remove the drvdata entirely.
> 
For some dummy sources, it's needed to allocate traceid for them in 
kernel driver, so this struct would be useful for that case.

I will remove it now and upstream it in the further.

Thanks,
Hao

> Otherwise looks good to me
> 
> Suzuki
> 

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

[Hao Zhang]

Hi Suzuki, Leo,

On 5/5/2023 8:05 PM, Leo Yan wrote:
> On Fri, May 05, 2023 at 11:54:03AM +0100, Suzuki Kuruppassery Poulose wrote:
> 
> [...]
> 
>>> +title: ARM Coresight Dummy component
>>> +
>>> +description: |
>>> +  Coresight Dummy Trace Module is for the specific devices that kernel
>>> +  don't have permission to access or configure, e.g., CoreSight TPDMs
>>> +  on Qualcomm platforms. So there need driver to register dummy devices
>>> +  as Coresight devices. It may also be used to define components that
>>> +  may not have any programming interfaces (e.g, static links), so that
>>> +  paths can be established in the driver. Provide Coresight API for
>>> +  dummy device operations, such as enabling and disabling dummy devices.
>>> +  Build the Coresight path for dummy sink or dummy source for debugging. > +
>>> +  The primary use case of the coresight dummy is to build path in kernel
>>> +  side for dummy sink and dummy source.
>>> +
>>> +maintainers:
>>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
>>> +  - Tao Zhang <quic_taozha@quicinc.com>
>>> +  - Hao Zhang <quic_hazha@quicinc.com>
>>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
>>
>> Given this is a generic "CoreSight" component, I would prefer to have the
>> CoreSight subsystem maintainers listed here (too). I don't mind
>> the entries above, but would like to make sure that the subsystem
>> people are aware of the changes happening here. Please use:
>>
>> Mike Leach <mike.leach@linaro.org>
>> Suzuki K Poulose <suzuki.poulose@arm.com>
>> Leo Yan <leo.yan@linaro.org>
>  > Given I am spending little time on CoreSight reviewing, I'd like to
> use James Clark's email address to replace my own; I believe this
> would benefit long term maintenance.
> 
>    James Clark <james.clark@arm.com>
> 
> Thanks!
>
Thanks for your review. I will update the maintainers in the next patch 
series.

Thanks,
Hao

>> With the above:
>>
>> Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - arm,coresight-dummy-sink
>>> +          - arm,coresight-dummy-source
>>> +
>>> +  out-ports:
>>> +    $ref: /schemas/graph.yaml#/properties/ports
>>> +
>>> +    properties:
>>> +      port:
>>> +        description: Output connection from the source to Coresight
>>> +          Trace bus.
>>> +        $ref: /schemas/graph.yaml#/properties/port
>>> +
>>> +  in-ports:
>>> +    $ref: /schemas/graph.yaml#/properties/ports
>>> +
>>> +    properties:
>>> +      port:
>>> +        description: Input connection from the Coresight Trace bus to
>>> +          dummy sink, such as Embedded USB debugger(EUD).
>>> +        $ref: /schemas/graph.yaml#/properties/port
>>> +
>>> +required:
>>> +  - compatible
>>> +
>>> +if:
>>> +  # If the compatible contains the below value
>>> +  properties:
>>> +    compatible:
>>> +      contains:
>>> +        const: arm,coresight-dummy-sink
>>> +
>>> +then:
>>> +  required:
>>> +    - in-ports
>>> +
>>> +else:
>>> +  required:
>>> +    - out-ports
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  # Minimum dummy sink definition. Dummy sink connect to coresight replicator.
>>> +  - |
>>> +    sink {
>>> +      compatible = "arm,coresight-dummy-sink";
>>> +
>>> +      in-ports {
>>> +        port {
>>> +          eud_in_replicator_swao: endpoint {
>>> +            remote-endpoint = <&replicator_swao_out_eud>;
>>> +          };
>>> +        };
>>> +      };
>>> +    };
>>> +
>>> +  # Minimum dummy source definition. Dummy source connect to coresight funnel.
>>> +  - |
>>> +    source {
>>> +      compatible = "arm,coresight-dummy-source";
>>> +
>>> +      out-ports {
>>> +        port {
>>> +          dummy_riscv_out_funnel_swao: endpoint {
>>> +            remote-endpoint = <&funnel_swao_in_dummy_riscv>;
>>> +          };
>>> +        };
>>> +      };
>>> +    };
>>> +
>>> +...
>>

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

[Hao Zhang]

On 5/7/2023 4:04 PM, Krzysztof Kozlowski wrote:
> On 05/05/2023 11:24, Hao Zhang wrote:
>> Add new coresight-dummy.yaml file describing the bindings required
>> to define coresight dummy trace in the device trees.
>>
>> Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
>> ---
>>   .../bindings/arm/arm,coresight-dummy.yaml     | 102 ++++++++++++++++++
>>   1 file changed, 102 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
>> new file mode 100644
>> index 000000000000..126518863eea
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
>> @@ -0,0 +1,102 @@
>> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/arm/arm,coresight-dummy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: ARM Coresight Dummy component
>> +
>> +description: |
>> +  Coresight Dummy Trace Module is for the specific devices that kernel
>> +  don't have permission to access or configure, e.g., CoreSight TPDMs
>> +  on Qualcomm platforms. So there need driver to register dummy devices
>> +  as Coresight devices. It may also be used to define components that
>> +  may not have any programming interfaces (e.g, static links), so that
>> +  paths can be established in the driver. Provide Coresight API for
>> +  dummy device operations, such as enabling and disabling dummy devices.
>> +  Build the Coresight path for dummy sink or dummy source for debugging.
>> +
>> +  The primary use case of the coresight dummy is to build path in kernel
>> +  side for dummy sink and dummy source.
>> +
>> +maintainers:
>> +  - Mao Jinlong <quic_jinlmao@quicinc.com>
>> +  - Tao Zhang <quic_taozha@quicinc.com>
>> +  - Hao Zhang <quic_hazha@quicinc.com>
>> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
>> +
>> +properties:
>> +  compatible:
>> +    items:
> 
> You were asked to drop oneOf, not to replace with items. Drop items.
> Drop oneOf. It's just enum.
> 

Hi Krzysztof,

I will drop items and update it in the next version.

Thanks,
Hao

>> +      - enum:
>> +          - arm,coresight-dummy-sink
>> +          - arm,coresight-dummy-source
>> +
>> +  out-ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      port:
>> +        description: Output connection from the source to Coresight
>> +          Trace bus.
>> +        $ref: /schemas/graph.yaml#/properties/port
>> +
>> +  in-ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      port:
>> +        description: Input connection from the Coresight Trace bus to
>> +          dummy sink, such as Embedded USB debugger(EUD).
>> +        $ref: /schemas/graph.yaml#/properties/port
>> +
>> +required:
>> +  - compatible
>> +
>> +if:
>> +  # If the compatible contains the below value
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        const: arm,coresight-dummy-sink
>> +
>> +then:
>> +  required:
>> +    - in-ports
>> +
>> +else:
>> +  required:
>> +    - out-ports
> 
> No improvements. Implement Rob's comments.
> 

Hi Krzysztof, Rob,

There are two comments from Rob:
1) I could imagine the OS wanting to know more information than just
'dummy'. Is data from an unknown source useful? Likewise, don't you want
to know where you are sending data too?
2) This still allows the nodes when they don't make sense. I think this
needs to be 2 schema files. The only common part is 'compatible' and
that's not even shared.


1. The necessary information for coresight is connection(ports) between 
different components. However, this information is not very intuitive. 
There would be a generic change to support coresight name in the 
further. You can refer to the below link, this solution is still under 
discussion, I think it's also helpful for our query.
https://lore.kernel.org/linux-arm-kernel/b7abee2a-99ca-26d6-5850-60ee19d9c0e9@quicinc.com/T/

2. Dummy driver is very simple, the only goal of it is to build a path 
in kernel for subsystem, so we want to handle dummy source and sink in 
one generic driver. For one same driver, shall we split the schema file?

Please feel free to comment.

Thanks,
Hao

> Best regards,
> Krzysztof
> 

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

[Hao Zhang]

On 5/10/2023 10:56 AM, Hao Zhang wrote:
> Hi Suzuki,
> 
> On 5/5/2023 5:57 PM, Suzuki K Poulose wrote:
>> On 05/05/2023 10:24, Hao Zhang wrote:
>>> Some Coresight devices that kernel don't have permission to access or
>>> configure. So there need driver to register dummy devices as Coresight
>>> devices. It may also be used to define components that may not have
>>> any programming interfaces (e.g, static links), so that paths can be
>>> established in the driver. Provide Coresight API for dummy device
>>> operations, such as enabling and disabling dummy devices. Build the
>>> Coresight path for dummy sink or dummy source for debugging.
>>>
>>> Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
>>> ---
>>>   drivers/hwtracing/coresight/Kconfig           |  11 ++
>>>   drivers/hwtracing/coresight/Makefile          |   1 +
>>>   drivers/hwtracing/coresight/coresight-dummy.c | 171 ++++++++++++++++++
>>>   include/linux/coresight.h                     |   1 +
>>>   4 files changed, 184 insertions(+)
>>>   create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c
>>>
>>> diff --git a/drivers/hwtracing/coresight/Kconfig 
>>> b/drivers/hwtracing/coresight/Kconfig
>>> index 2b5bbfffbc4f..06f0a7594169 100644
>>> --- a/drivers/hwtracing/coresight/Kconfig
>>> +++ b/drivers/hwtracing/coresight/Kconfig
>>> @@ -236,4 +236,15 @@ config CORESIGHT_TPDA
>>>         To compile this driver as a module, choose M here: the module 
>>> will be
>>>         called coresight-tpda.
>>> +
>>> +config CORESIGHT_DUMMY
>>> +    tristate "Dummy driver support"
>>> +    help
>>> +      Enables support for dummy driver. Dummy driver can be used for
>>> +      CoreSight sources/sinks that are owned and configured by some
>>> +      other subsystem and use Linux drivers to configure rest of trace
>>> +      path > +
>>> +      To compile this driver as a module, choose M here: the module 
>>> will be
>>> +      called coresight-dummy.
>>>   endif
>>> diff --git a/drivers/hwtracing/coresight/Makefile 
>>> b/drivers/hwtracing/coresight/Makefile
>>> index 33bcc3f7b8ae..995d3b2c76df 100644
>>> --- a/drivers/hwtracing/coresight/Makefile
>>> +++ b/drivers/hwtracing/coresight/Makefile
>>> @@ -30,3 +30,4 @@ obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o
>>>   coresight-cti-y := coresight-cti-core.o    coresight-cti-platform.o \
>>>              coresight-cti-sysfs.o
>>>   obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
>>> +obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
>>> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c 
>>> b/drivers/hwtracing/coresight/coresight-dummy.c
>>> new file mode 100644
>>> index 000000000000..ee9881ff4754
>>> --- /dev/null
>>> +++ b/drivers/hwtracing/coresight/coresight-dummy.c
>>> @@ -0,0 +1,171 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights 
>>> reserved.
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/coresight.h>
>>> +#include <linux/of.h>
>>> +#include <linux/pm_runtime.h>
>>
>> Please follow the alphabetical order for the header files ^
>>
> Sure, I will update this in the next patch series.
> 
>>> +
>>> +#include "coresight-priv.h"
>>> +
>>> +struct dummy_drvdata {
>>> +    struct device            *dev;
>>
>> nit: We don't need this really. And that completely removes the need for
>> drvdata too. See below.
>>
>>> +    struct coresight_device        *csdev;
>>> +};
>>> +
>>> +DEFINE_CORESIGHT_DEVLIST(source_devs, "dummy_source");
>>> +DEFINE_CORESIGHT_DEVLIST(sink_devs, "dummy_sink");
>>> +
>>> +static int dummy_source_enable(struct coresight_device *csdev,
>>> +                   struct perf_event *event, u32 mode)
>>> +{
>>> +    struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>> +
>>> +    dev_dbg(drvdata->dev, "Dummy source enabled\n");
>>
>>      dev_dbg(csdev->dev.parent, ..");
>>
>> Similarly for all instances below.
>>
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void dummy_source_disable(struct coresight_device *csdev,
>>> +                 struct perf_event *event)
>>> +{
>>> +    struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>> +
>>> +    dev_dbg(drvdata->dev, "Dummy source disabled\n");
>>> +}
>>> +
>>> +static int dummy_sink_enable(struct coresight_device *csdev, u32 mode,
>>> +                void *data)
>>> +{
>>> +    struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>> +
>>> +    dev_dbg(drvdata->dev, "Dummy sink enabled\n");
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int dummy_sink_disable(struct coresight_device *csdev)
>>> +{
>>> +    struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>> +
>>> +    dev_dbg(drvdata->dev, "Dummy sink disabled\n");
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct coresight_ops_source dummy_source_ops = {
>>> +    .enable    = dummy_source_enable,
>>> +    .disable = dummy_source_disable,
>>> +};
>>> +
>>> +static const struct coresight_ops dummy_source_cs_ops = {
>>> +    .source_ops = &dummy_source_ops,
>>> +};
>>> +
>>> +static const struct coresight_ops_sink dummy_sink_ops = {
>>> +    .enable    = dummy_sink_enable,
>>> +    .disable = dummy_sink_disable,
>>> +};
>>> +
>>> +static const struct coresight_ops dummy_sink_cs_ops = {
>>> +    .sink_ops = &dummy_sink_ops,
>>> +};
>>> +
>>> +static int dummy_probe(struct platform_device *pdev)
>>> +{
>>> +    struct device *dev = &pdev->dev;
>>> +    struct device_node *node = dev->of_node;
>>> +    struct coresight_platform_data *pdata;
>>> +    struct dummy_drvdata *drvdata;
>>> +    struct coresight_desc desc = { 0 };
>>> +
>>> +    if (of_device_is_compatible(node, "arm,coresight-dummy-source")) {
>>> +
>>> +        desc.name = coresight_alloc_device_name(&source_devs, dev);
>>> +        if (!desc.name)
>>> +            return -ENOMEM;
>>> +
>>> +        desc.type = CORESIGHT_DEV_TYPE_SOURCE;
>>> +        desc.subtype.source_subtype =
>>> +                    CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS;
>>> +        desc.ops = &dummy_source_cs_ops;
>>> +    } else if (of_device_is_compatible(node, 
>>> "arm,coresight-dummy-sink")) {
>>> +        desc.name = coresight_alloc_device_name(&sink_devs, dev);
>>> +        if (!desc.name)
>>> +            return -ENOMEM;
>>> +
>>> +        desc.type = CORESIGHT_DEV_TYPE_SINK;
>>> +        desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_DUMMY;
>>> +        desc.ops = &dummy_sink_cs_ops;
>>> +    } else {
>>> +        dev_err(dev, "Device type not set\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    pdata = coresight_get_platform_data(dev);
>>> +    if (IS_ERR(pdata))
>>> +        return PTR_ERR(pdata);
>>> +    pdev->dev.platform_data = pdata;
>>> +
>>> +    drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>>> +    if (!drvdata)
>>> +        return -ENOMEM; > +
>>> +    drvdata->dev = &pdev->dev;
>>> +    platform_set_drvdata(pdev, drvdata);
>>
>> As above, you may remove the drvdata entirely.
>>
> For some dummy sources, it's needed to allocate traceid for them in 
> kernel driver, so this struct would be useful for that case.
> 
> I will remove it now and upstream it in the further.
> 

Hi Suzuki,

After checking, I find the drvdata struct is required for csdev which 
would store the pointer of coresight_device, and it is also required for 
traceid in the further. So I will keep drvdata in the next patch series.

Thanks,
Hao

> Thanks,
> Hao
> 
>> Otherwise looks good to me
>>
>> Suzuki
>>

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

[Rob Herring]

On Mon, May 15, 2023 at 01:52:41PM +0800, Hao Zhang wrote:
> 
> 
> On 5/7/2023 4:04 PM, Krzysztof Kozlowski wrote:
> > On 05/05/2023 11:24, Hao Zhang wrote:
> > > Add new coresight-dummy.yaml file describing the bindings required
> > > to define coresight dummy trace in the device trees.
> > > 
> > > Signed-off-by: Hao Zhang <quic_hazha@quicinc.com>
> > > ---
> > >   .../bindings/arm/arm,coresight-dummy.yaml     | 102 ++++++++++++++++++
> > >   1 file changed, 102 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
> > > new file mode 100644
> > > index 000000000000..126518863eea
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/arm/arm,coresight-dummy.yaml
> > > @@ -0,0 +1,102 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/arm/arm,coresight-dummy.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ARM Coresight Dummy component
> > > +
> > > +description: |
> > > +  Coresight Dummy Trace Module is for the specific devices that kernel
> > > +  don't have permission to access or configure, e.g., CoreSight TPDMs
> > > +  on Qualcomm platforms. So there need driver to register dummy devices
> > > +  as Coresight devices. It may also be used to define components that
> > > +  may not have any programming interfaces (e.g, static links), so that
> > > +  paths can be established in the driver. Provide Coresight API for
> > > +  dummy device operations, such as enabling and disabling dummy devices.
> > > +  Build the Coresight path for dummy sink or dummy source for debugging.
> > > +
> > > +  The primary use case of the coresight dummy is to build path in kernel
> > > +  side for dummy sink and dummy source.
> > > +
> > > +maintainers:
> > > +  - Mao Jinlong <quic_jinlmao@quicinc.com>
> > > +  - Tao Zhang <quic_taozha@quicinc.com>
> > > +  - Hao Zhang <quic_hazha@quicinc.com>
> > > +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > 
> > You were asked to drop oneOf, not to replace with items. Drop items.
> > Drop oneOf. It's just enum.
> > 
> 
> Hi Krzysztof,
> 
> I will drop items and update it in the next version.
> 
> Thanks,
> Hao
> 
> > > +      - enum:
> > > +          - arm,coresight-dummy-sink
> > > +          - arm,coresight-dummy-source
> > > +
> > > +  out-ports:
> > > +    $ref: /schemas/graph.yaml#/properties/ports
> > > +
> > > +    properties:
> > > +      port:
> > > +        description: Output connection from the source to Coresight
> > > +          Trace bus.
> > > +        $ref: /schemas/graph.yaml#/properties/port
> > > +
> > > +  in-ports:
> > > +    $ref: /schemas/graph.yaml#/properties/ports
> > > +
> > > +    properties:
> > > +      port:
> > > +        description: Input connection from the Coresight Trace bus to
> > > +          dummy sink, such as Embedded USB debugger(EUD).
> > > +        $ref: /schemas/graph.yaml#/properties/port
> > > +
> > > +required:
> > > +  - compatible
> > > +
> > > +if:
> > > +  # If the compatible contains the below value
> > > +  properties:
> > > +    compatible:
> > > +      contains:
> > > +        const: arm,coresight-dummy-sink
> > > +
> > > +then:
> > > +  required:
> > > +    - in-ports
> > > +
> > > +else:
> > > +  required:
> > > +    - out-ports
> > 
> > No improvements. Implement Rob's comments.
> > 
> 
> Hi Krzysztof, Rob,
> 
> There are two comments from Rob:
> 1) I could imagine the OS wanting to know more information than just
> 'dummy'. Is data from an unknown source useful? Likewise, don't you want
> to know where you are sending data too?
> 2) This still allows the nodes when they don't make sense. I think this
> needs to be 2 schema files. The only common part is 'compatible' and
> that's not even shared.
> 
> 
> 1. The necessary information for coresight is connection(ports) between
> different components. However, this information is not very intuitive. There
> would be a generic change to support coresight name in the further. You can
> refer to the below link, this solution is still under discussion, I think
> it's also helpful for our query.
> https://lore.kernel.org/linux-arm-kernel/b7abee2a-99ca-26d6-5850-60ee19d9c0e9@quicinc.com/T/

I don't really have more input. I'm looking for input from coresight 
folks on this.

Design the binding so it is future proof. If you need to go back and add 
more data to the binding in the future you've lost. It is perfectly fine 
to have specific bindings with generic drivers (e.g. simple panel). Then 
if you need more capabilities than a generic driver, then you can add a 
specific driver in the future with no DT change.

> 2. Dummy driver is very simple, the only goal of it is to build a path in
> kernel for subsystem, so we want to handle dummy source and sink in one
> generic driver. For one same driver, shall we split the schema file?

schemas and drivers are not 1:1. So yes, split the schema.

Rob