On 24/03/2023 14:58, Tao Zhang wrote:
Hi Suzuki,
在 3/23/2023 7:51 PM, Suzuki K Poulose 写道:
On 23/03/2023 06:03, 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-tpda.c | 58
++++++++++++++++++++++++++++
drivers/hwtracing/coresight/coresight-tpda.h | 4 ++
2 files changed, 62 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c
b/drivers/hwtracing/coresight/coresight-tpda.c
index f712e11..8dcfc4a 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -21,6 +21,47 @@
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.
+ */
I am still not clear why we need to do this recursively ?
Some TPDMs are not directly output connected to the TPDAs. So here I
use a recursive method to check from the TPDA input port until I find
the connected TPDM.
Do you have a better suggestion besides a recursive method?
+static int tpda_set_element_size(struct tpda_drvdata *drvdata,
+ struct coresight_device *csdev, int inport, bool parent)
Please could we renamse csdev => tpda_dev
Since this is a recursively called function, this Coresight device is not
necessarily TPDA, it can be other Coresight device.
+{
+ static int nr_inport;
+ int i;
+ struct coresight_device *in_csdev;
similarly tpdm_dev ?
Same as above, this variable may not necessarily be a TPDM.
Could we not add a check here to see if the dsb_esize[inport] is already
set and then bail out, reading this over and over ?
I will update this in the next patch series.
+
+ if (inport > (TPDA_MAX_INPORTS - 1))
+ return -EINVAL;
+
+ if (parent)
+ nr_inport = inport;
+
+ for (i = 0; i < csdev->pdata->nr_inconns; i++) {
+ in_csdev = csdev->pdata->in_conns[i].remote_dev;
Please note, the names of the structure field might change in the
next version of James' series
Got it. I will keep an eye out for the James' patch series.
+ if (!in_csdev)
+ break;
+
+ if (parent)
+ if (csdev->pdata->in_conns[i].port != inport)
+ continue;
+
+ if (in_csdev && strstr(dev_name(&in_csdev->dev), "tpdm")) {
Isn't there a better way to distinguish a device to be TPDM ? May be we
could even add a source_sub_type - SOURCE_TPDM instead of using
SOURCE_OTHERS ? Do you expect other sources to be connected to TPDA?
e.g., STMs ?
I can add "SOURCE_TPDM" as a source_sub_type, but SOURCE_OTHERS needs
to be kept since the other Coresight component we will upstream later may
need it.
+ of_property_read_u32(in_csdev->dev.parent->of_node,
+ "qcom,dsb-element-size",
&drvdata->dsb_esize[nr_inport]);
+ break;
+ }
+ tpda_set_element_size(drvdata, in_csdev, 0, false);
What is the point of this ? Is this for covering the a TPDA connected to
another TPDA ?
e.g., { TPDM0, TPDM1 } -> TPDA0 -> TPDA1 ?
A TPDM may not connect to the TPDA directly, for example,
TPDM0 ->FUNNEL0->FUNNEL1->TPDA0
And many TPDMs can connect to one TPDA, one input port on TPDA only has
one TPDM connected. Therefore, we use a recursive method to find the TPDM
corresponding to the input port of TPDA.
How do you find out decide what to choose, if there are multiple TPDMs
connected to FUNNEL0 or even FUNNEL1 ?
e.g
TPDM0->FUNNEL0->FUNNEL1->TPDA0
/
TPDM1
Suzuki
