Re: [PATCH v4 02/11] coresight-tpda: Add DSB dataset support

From: Tao Zhang
Date: Thu May 25 2023 - 23:22:37 EST



On 5/25/2023 5:08 PM, Suzuki K Poulose wrote:
On 25/05/2023 08:16, Tao Zhang wrote:

On 5/23/2023 6:07 PM, Suzuki K Poulose wrote:
On 27/04/2023 10:00, Tao Zhang wrote:
Read the DSB element size from the device tree. Set the register
bit that controls the DSB element size of the corresponding port.

Signed-off-by: Tao Zhang <quic_taozha@xxxxxxxxxxx>
---
  drivers/hwtracing/coresight/coresight-core.c |  1 +
  drivers/hwtracing/coresight/coresight-tpda.c | 92 +++++++++++++++++++++++++---
  drivers/hwtracing/coresight/coresight-tpda.h |  4 ++
  drivers/hwtracing/coresight/coresight-tpdm.c |  2 +-
  include/linux/coresight.h                    |  1 +
  5 files changed, 90 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 2af416b..f1eacbb 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1092,6 +1092,7 @@ 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_TPDM &&
          subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS) {
          dev_err(&csdev->dev, "wrong device subtype in %s\n", function);
          return -EINVAL;

Please see the comment at the bottom.

diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 8d2b9d2..af9c72f 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -21,6 +21,56 @@
    DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
  +/* Search and read element data size from the TPDM node in
+ * the devicetree. Each input port of TPDA is connected to
+ * a TPDM. Different TPDM supports different types of dataset,
+ * and some may support more than one type of dataset.
+ * Parameter "inport" is used to pass in the input port number
+ * of TPDA, and it is set to 0 in the recursize call.
+ * Parameter "parent" is used to pass in the original call.
+ */
+static int tpda_set_element_size(struct tpda_drvdata *drvdata,
+               struct coresight_device *csdev, int inport, bool parent)
+{
+    static int nr_inport;
+    int i;
+    static bool tpdm_found;
+    struct coresight_device *in_csdev;
+
+    if (inport > (TPDA_MAX_INPORTS - 1))
+        return -EINVAL;
+
+    if (parent) {
+        nr_inport = inport;
+        tpdm_found = false;
+    }
+
+    for (i = 0; i < csdev->pdata->nr_inconns; i++) {
+        in_csdev = csdev->pdata->in_conns[i]->src_dev;
+        if (!in_csdev)
+            break;
+
+        if (parent)
+            if (csdev->pdata->in_conns[i]->dest_port != inport)
+                continue;
+
+        if (in_csdev->subtype.source_subtype

We must match the in_csdev->type to be SOURCE && the subtype.
Sure, I will update it in the next patch series.

+                   == CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM) {
+ of_property_read_u8(in_csdev->dev.parent->of_node,
+                    "qcom,dsb-element-size", &drvdata->dsb_esize[nr_inport]);
+            if (!tpdm_found)
+                tpdm_found = true;
+            else
+                dev_warn(drvdata->dev,
+                    "More than one TPDM is mapped to the TPDA input port %d.\n",
+                    nr_inport);

When we know, we have found a source device, we don't need to recurse
down and could simply 'continue' to the next one in the list and skip
the call below.

Actually, one input port on TPDA only can connect to one TPDM. In the current design, it will

find out all the TPDMs on one input port and warn the users all the TPDMs it found. If we

replace 'recurse down' as 'continue' here, it may not find some TPDMs that might be connected

incorrectly.


What do you mean ? When you enter the if () above, the in_csdev is a
source and it is TPDM. There must be no input connections TPDM, i.e.
in_csdev, so no need to go further up the connection chain looking at
the in_csdev. The loop will continue to analyse this device (where we
found one TPDM already) and detect any further duplicate TPDMs
connected.

Got it. You're right.

I will update this in the next patch series as well.


Suzuki