2021-05-10 12:06:49

by Tao Zhang

[permalink] [raw]
Subject: [PATCH v1 0/3] coresight: Support for building more coresight paths

We are trying to achieve more types of Coresight source devices.
For example, we have a type of coresight source devic named TPDM.
In the process of using, sometimes mulitiple TPDMs need to be
connected to the different input ports on the same funnel.
Meanwhile, these TPDMs also need to output from different
ports on the funnel.
But, at present the Coresight driver assumes
a) Only support Coresight source type ETM, ETR and ETF
b) Funnels only support mulitiple inputs and one output
Which doesn't help to add the above feature for our new Coresight
source device TPDM. So, in order to accommodate the new device,
we develop the following patches.
a) Add support more types of Coresight source devices.
b) Add support for multiple output ports on funnel and the output
ports could be selected by Corsight source.

Applies on coresight/next.
http://git.linaro.org/kernel/coresight.git/log/?h=next

Tao Zhang (3):
coresight: add support to enable more coresight paths
coresight: funnel: add support for multiple output ports
dt-bindings: arm: add property for selecting funnel output port

.../devicetree/bindings/arm/coresight.txt | 5 +
drivers/hwtracing/coresight/coresight-core.c | 169 ++++++++++++++-------
drivers/hwtracing/coresight/coresight-platform.c | 9 ++
include/linux/coresight.h | 2 +
4 files changed, 127 insertions(+), 58 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2021-05-10 12:07:25

by Tao Zhang

[permalink] [raw]
Subject: [PATCH v1 2/3] coresight: funnel: add support for multiple output ports

Funnel devices are now capable of supporting multiple-inputs and
multiple-outputs configuration with in built hardware filtering
for TPDM devices. Add software support to this function. Output
ports is selected according to the source of the trace path.

Signed-off-by: Tingwei Zhang <[email protected]>
Signed-off-by: Tao Zhang <[email protected]>
---
drivers/hwtracing/coresight/coresight-core.c | 68 +++++++++++++++++++-----
drivers/hwtracing/coresight/coresight-platform.c | 9 ++++
include/linux/coresight.h | 2 +
3 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 7dfadb6..e14b294 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -54,6 +54,8 @@ static LIST_HEAD(cs_active_paths);
const u32 coresight_barrier_pkt[4] = {0x7fffffff, 0x7fffffff, 0x7fffffff, 0x7fffffff};
EXPORT_SYMBOL_GPL(coresight_barrier_pkt);

+static struct coresight_device *coresight_get_source(struct list_head *path);
+
static const struct cti_assoc_op *cti_assoc_ops;

void coresight_set_cti_ops(const struct cti_assoc_op *cti_op)
@@ -107,14 +109,42 @@ static int coresight_source_is_unique(struct coresight_device *csdev)
csdev, coresight_id_match);
}

+/**
+ * coresight_source_filter - checks whether the connection matches the source
+ * of path if connection is binded to specific source.
+ * @path: The list of devices
+ * @conn: The connection of one outport
+ *
+ * Return zero if the connection doesn't have a source binded or source of the
+ * path matches the source binds to connection.
+ */
+static int coresight_source_filter(struct list_head *path,
+ struct coresight_connection *conn)
+{
+ int ret = 0;
+ struct coresight_device *source = NULL;
+
+ if (conn->source_name == NULL)
+ return ret;
+
+ source = coresight_get_source(path);
+ if (source == NULL)
+ return ret;
+
+ return strcmp(conn->source_name, dev_name(&source->dev));
+}
+
static int coresight_find_link_inport(struct coresight_device *csdev,
- struct coresight_device *parent)
+ struct coresight_device *parent,
+ struct list_head *path)
{
int i;
struct coresight_connection *conn;

for (i = 0; i < parent->pdata->nr_outport; i++) {
conn = &parent->pdata->conns[i];
+ if (coresight_source_filter(path, conn))
+ continue;
if (conn->child_dev == csdev)
return conn->child_port;
}
@@ -126,13 +156,16 @@ static int coresight_find_link_inport(struct coresight_device *csdev,
}

static int coresight_find_link_outport(struct coresight_device *csdev,
- struct coresight_device *child)
+ struct coresight_device *child,
+ struct list_head *path)
{
int i;
struct coresight_connection *conn;

for (i = 0; i < csdev->pdata->nr_outport; i++) {
conn = &csdev->pdata->conns[i];
+ if (coresight_source_filter(path, conn))
+ continue;
if (conn->child_dev == child)
return conn->outport;
}
@@ -329,7 +362,8 @@ static void coresight_disable_sink(struct coresight_device *csdev)

static int coresight_enable_link(struct coresight_device *csdev,
struct coresight_device *parent,
- struct coresight_device *child)
+ struct coresight_device *child,
+ struct list_head *path)
{
int ret = 0;
int link_subtype;
@@ -338,8 +372,8 @@ static int coresight_enable_link(struct coresight_device *csdev,
if (!parent || !child)
return -EINVAL;

- inport = coresight_find_link_inport(csdev, parent);
- outport = coresight_find_link_outport(csdev, child);
+ inport = coresight_find_link_inport(csdev, parent, path);
+ outport = coresight_find_link_outport(csdev, child, path);
link_subtype = csdev->subtype.link_subtype;

if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && inport < 0)
@@ -364,7 +398,8 @@ static int coresight_enable_link(struct coresight_device *csdev,

static void coresight_disable_link(struct coresight_device *csdev,
struct coresight_device *parent,
- struct coresight_device *child)
+ struct coresight_device *child,
+ struct list_head *path)
{
int i, nr_conns;
int link_subtype;
@@ -373,8 +408,8 @@ static void coresight_disable_link(struct coresight_device *csdev,
if (!parent || !child)
return;

- inport = coresight_find_link_inport(csdev, parent);
- outport = coresight_find_link_outport(csdev, child);
+ inport = coresight_find_link_inport(csdev, parent, path);
+ outport = coresight_find_link_outport(csdev, child, path);
link_subtype = csdev->subtype.link_subtype;

if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG) {
@@ -489,7 +524,7 @@ static void coresight_disable_path_from(struct list_head *path,
case CORESIGHT_DEV_TYPE_LINK:
parent = list_prev_entry(nd, link)->csdev;
child = list_next_entry(nd, link)->csdev;
- coresight_disable_link(csdev, parent, child);
+ coresight_disable_link(csdev, parent, child, path);
break;
default:
break;
@@ -544,7 +579,7 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data)
case CORESIGHT_DEV_TYPE_LINK:
parent = list_prev_entry(nd, link)->csdev;
child = list_next_entry(nd, link)->csdev;
- ret = coresight_enable_link(csdev, parent, child);
+ ret = coresight_enable_link(csdev, parent, child, path);
if (ret)
goto err;
break;
@@ -772,7 +807,8 @@ static void coresight_drop_device(struct coresight_device *csdev)
*/
static int _coresight_build_path(struct coresight_device *csdev,
struct coresight_device *sink,
- struct list_head *path)
+ struct list_head *path,
+ struct coresight_device *source)
{
int i, ret;
bool found = false;
@@ -787,8 +823,14 @@ static int _coresight_build_path(struct coresight_device *csdev,
struct coresight_device *child_dev;

child_dev = csdev->pdata->conns[i].child_dev;
+
+ if (csdev->pdata->conns[i].source_name &&
+ strcmp(csdev->pdata->conns[i].source_name,
+ dev_name(&source->dev)))
+ continue;
+
if (child_dev &&
- _coresight_build_path(child_dev, sink, path) == 0) {
+ _coresight_build_path(child_dev, sink, path, source) == 0) {
found = true;
break;
}
@@ -833,7 +875,7 @@ struct list_head *coresight_build_path(struct coresight_device *source,

INIT_LIST_HEAD(path);

- rc = _coresight_build_path(source, sink, path);
+ rc = _coresight_build_path(source, sink, path, source);
if (rc) {
kfree(path);
return ERR_PTR(rc);
diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
index 3629b78..47c453f 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -216,6 +216,7 @@ static int of_coresight_parse_endpoint(struct device *dev,
struct of_endpoint endpoint, rendpoint;
struct device_node *rparent = NULL;
struct device_node *rep = NULL;
+ struct device_node *sn = NULL;
struct device *rdev = NULL;
struct fwnode_handle *rdev_fwnode;
struct coresight_connection *conn;
@@ -263,6 +264,14 @@ static int of_coresight_parse_endpoint(struct device *dev,
*/
conn->child_fwnode = fwnode_handle_get(rdev_fwnode);
conn->child_port = rendpoint.port;
+ conn->source_name = NULL;
+ sn = of_parse_phandle(ep, "source", 0);
+ if (sn) {
+ ret = of_property_read_string(sn,
+ "coresight-name", &conn->source_name);
+ of_node_put(sn);
+ }
+
/* Connection record updated */
} while (0);

diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 7d3c87e..ccc2c86 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -140,6 +140,7 @@ struct coresight_desc {
* struct coresight_connection - representation of a single connection
* @outport: a connection's output port number.
* @child_port: remote component's port number @output is connected to.
+ * @source_name:source component's name.
* @chid_fwnode: remote component's fwnode handle.
* @child_dev: a @coresight_device representation of the component
connected to @outport.
@@ -148,6 +149,7 @@ struct coresight_desc {
struct coresight_connection {
int outport;
int child_port;
+ const char *source_name;
struct fwnode_handle *child_fwnode;
struct coresight_device *child_dev;
struct coresight_sysfs_link *link;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2021-05-10 12:17:40

by Tao Zhang

[permalink] [raw]
Subject: [PATCH v1 1/3] coresight: add support to enable more coresight paths

Current coresight implementation only supports enabling source
ETMs or STM. This patch adds support to enable more kinds of
coresight source to sink paths. We build a path from source to
sink when any source is enabled and store it in a list. When the
source is disabled, we fetch the corresponding path from the list
and decrement the refcount on each device in the path. The device
is disabled if the refcount reaches zero. Don't store path to
coresight data structure of source to avoid unnecessary change to
ABI.
Since some targets may have coresight sources other than STM and
ETMs, we need to add this change to support these coresight
devices.

Signed-off-by: Satyajit Desai <[email protected]>
Signed-off-by: Rama Aparna Mallavarapu <[email protected]>
Signed-off-by: Mulu He <[email protected]>
Signed-off-by: Tingwei Zhang <[email protected]>
Signed-off-by: Tao Zhang <[email protected]>
---
drivers/hwtracing/coresight/coresight-core.c | 101 +++++++++++++++------------
1 file changed, 56 insertions(+), 45 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 4ba801d..7dfadb6 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -35,18 +35,16 @@ struct coresight_node {
};

/*
- * When operating Coresight drivers from the sysFS interface, only a single
- * path can exist from a tracer (associated to a CPU) to a sink.
+ * struct coresight_path - path from source to sink
+ * @path: Address of path list.
+ * @link: hook to the list.
*/
-static DEFINE_PER_CPU(struct list_head *, tracer_path);
+struct coresight_path {
+ struct list_head *path;
+ struct list_head link;
+};

-/*
- * As of this writing only a single STM can be found in CS topologies. Since
- * there is no way to know if we'll ever see more and what kind of
- * configuration they will enact, for the time being only define a single path
- * for STM.
- */
-static struct list_head *stm_path;
+static LIST_HEAD(cs_active_paths);

/*
* When losing synchronisation a new barrier packet needs to be inserted at the
@@ -326,7 +324,7 @@ static void coresight_disable_sink(struct coresight_device *csdev)
if (ret)
return;
coresight_control_assoc_ectdev(csdev, false);
- csdev->enable = false;
+ csdev->activated = false;
}

static int coresight_enable_link(struct coresight_device *csdev,
@@ -562,6 +560,20 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data)
goto out;
}

+static struct coresight_device *coresight_get_source(struct list_head *path)
+{
+ struct coresight_device *csdev;
+
+ if (!path)
+ return NULL;
+
+ csdev = list_first_entry(path, struct coresight_node, link)->csdev;
+ if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
+ return NULL;
+
+ return csdev;
+}
+
struct coresight_device *coresight_get_sink(struct list_head *path)
{
struct coresight_device *csdev;
@@ -1047,9 +1059,23 @@ static int coresight_validate_source(struct coresight_device *csdev,
return 0;
}

+static int coresight_store_path(struct list_head *path)
+{
+ struct coresight_path *node;
+
+ node = kzalloc(sizeof(struct coresight_path), GFP_KERNEL);
+ if (!node)
+ return -ENOMEM;
+
+ node->path = path;
+ list_add(&node->link, &cs_active_paths);
+
+ return 0;
+}
+
int coresight_enable(struct coresight_device *csdev)
{
- int cpu, ret = 0;
+ int ret = 0;
struct coresight_device *sink;
struct list_head *path;
enum coresight_dev_subtype_source subtype;
@@ -1094,25 +1120,9 @@ int coresight_enable(struct coresight_device *csdev)
if (ret)
goto err_source;

- switch (subtype) {
- case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
- /*
- * When working from sysFS it is important to keep track
- * of the paths that were created so that they can be
- * undone in 'coresight_disable()'. Since there can only
- * be a single session per tracer (when working from sysFS)
- * a per-cpu variable will do just fine.
- */
- cpu = source_ops(csdev)->cpu_id(csdev);
- per_cpu(tracer_path, cpu) = path;
- break;
- case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
- stm_path = path;
- break;
- default:
- /* We can't be here */
- break;
- }
+ ret = coresight_store_path(path);
+ if (ret)
+ goto err_source;

out:
mutex_unlock(&coresight_mutex);
@@ -1129,8 +1139,11 @@ EXPORT_SYMBOL_GPL(coresight_enable);

void coresight_disable(struct coresight_device *csdev)
{
- int cpu, ret;
+ int ret;
struct list_head *path = NULL;
+ struct coresight_path *cspath = NULL;
+ struct coresight_path *cspath_next = NULL;
+ struct coresight_device *src_csdev = NULL;

mutex_lock(&coresight_mutex);

@@ -1141,20 +1154,18 @@ void coresight_disable(struct coresight_device *csdev)
if (!csdev->enable || !coresight_disable_source(csdev))
goto out;

- switch (csdev->subtype.source_subtype) {
- case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
- cpu = source_ops(csdev)->cpu_id(csdev);
- path = per_cpu(tracer_path, cpu);
- per_cpu(tracer_path, cpu) = NULL;
- break;
- case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
- path = stm_path;
- stm_path = NULL;
- break;
- default:
- /* We can't be here */
- break;
+ list_for_each_entry_safe(cspath, cspath_next, &cs_active_paths, link) {
+ src_csdev = coresight_get_source(cspath->path);
+ if (!src_csdev)
+ continue;
+ if (src_csdev == csdev) {
+ path = cspath->path;
+ list_del(&cspath->link);
+ kfree(cspath);
+ }
}
+ if (path == NULL)
+ goto out;

coresight_disable_path(path);
coresight_release_path(path);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2021-05-10 13:16:34

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v1 0/3] coresight: Support for building more coresight paths

Hi Tao

On 10/05/2021 12:05, Tao Zhang wrote:
> We are trying to achieve more types of Coresight source devices.
> For example, we have a type of coresight source devic named TPDM.
> In the process of using, sometimes mulitiple TPDMs need to be
> connected to the different input ports on the same funnel.
> Meanwhile, these TPDMs also need to output from different
> ports on the funnel.
> But, at present the Coresight driver assumes
> a) Only support Coresight source type ETM, ETR and ETF

Did you mean ETM and STM here ? ETR & ETF are not source types, rather
they are SINK.


> b) Funnels only support mulitiple inputs and one output
> Which doesn't help to add the above feature for our new Coresight
> source device TPDM. So, in order to accommodate the new device,
> we develop the following patches.

Where is the TPDM driver ? Could you give us a rough idea of the
behavior in terms of the input / output ?


> a) Add support more types of Coresight source devices.

Which ones ? where is the code ?

> b) Add support for multiple output ports on funnel and the output
> ports could be selected by Corsight source.

Does the "TPDM" require programming to switch these output or are these
"static" ?

Is this something that can be avoided by having a "fake"
static-replicator in the path ?

e.g, TPDM
________________________________________________
In0 | | -> Out0
In1 | Static-Funnel -> Static-Replicator | -> Out1
In2 | | -> Out2
________________________________________________


Is this something that can be solved ? Again, please give a brief
description of the TPDM device and the driver code in the series to
give us a complete picture of what you are trying to do.

Reviewing some changes without having the full picture is not going to
be helpful.

Suzuki

2021-05-13 06:07:16

by Tao Zhang

[permalink] [raw]
Subject: Re: [PATCH v1 0/3] coresight: Support for building more coresight paths

On 2021-05-10 21:10, Suzuki K Poulose wrote:
> Hi Tao
>
> On 10/05/2021 12:05, Tao Zhang wrote:
>> We are trying to achieve more types of Coresight source devices.
>> For example, we have a type of coresight source devic named TPDM.
>> In the process of using, sometimes mulitiple TPDMs need to be
>> connected to the different input ports on the same funnel.
>> Meanwhile, these TPDMs also need to output from different
>> ports on the funnel.
>> But, at present the Coresight driver assumes
>> a) Only support Coresight source type ETM, ETR and ETF
>
> Did you mean ETM and STM here ? ETR & ETF are not source types, rather
> they are SINK.
>
>
Yes, I mean ETM and STM here.
>> b) Funnels only support mulitiple inputs and one output
>> Which doesn't help to add the above feature for our new Coresight
>> source device TPDM. So, in order to accommodate the new device,
>> we develop the following patches.
>
> Where is the TPDM driver ? Could you give us a rough idea of the
> behavior in terms of the input / output ?
>
>
We have plans to upload the TPDM driver in the feature. TPDM is a type
of
hardware component for debugging and monitoring on Qualcomm targets.
The primary use case of the TPDM is to collect data from different data
source and send it to a TPDA for packetization, timestamping and
funneling.
And the output of TPDA is a regular AMBA ATB stream and can be thought
of as
any other trace source in the system.
You can get a general understanding of the TPDM and TPDA driver through
the
following patch.
https://source.codeaurora.org/quic/la/kernel/msm-5.4/commit/?h=LV.AU.0.2.0.r1&id=a47c3313965c1101f2224e55da2c54d9e5c388dd
>> a) Add support more types of Coresight source devices.
>
> Which ones ? where is the code ?
>
In the patch
"0001-coresight-add-support-to-enable-more-coresight-paths.patch",
we replaced the original path save method with the funcation
"coresight_store_path".
In the original method, "coresight_enable" only can store the tracer
path for ETM
and STM. In the function "coresight_store_path", it can store the tracer
path for
more types of Coresight sources.
>> b) Add support for multiple output ports on funnel and the output
>> ports could be selected by Corsight source.
>
> Does the "TPDM" require programming to switch these output or are
> these "static" ?
>
> Is this something that can be avoided by having a "fake"
> static-replicator in the path ?
>
> e.g, TPDM
> ________________________________________________
> In0 | | -> Out0
> In1 | Static-Funnel -> Static-Replicator | -> Out1
> In2 | | -> Out2
> ________________________________________________
>
>
> Is this something that can be solved ? Again, please give a brief
> description of the TPDM device and the driver code in the series to
> give us a complete picture of what you are trying to do.
>
> Reviewing some changes without having the full picture is not going to
> be helpful.
>
> Suzuki
Now the link can support multiple out ports, but there are no entries
from
the link's device tree can figure out from which input port to which
output
port.
This patch provide the entry "source" in device tree, which can
associate
the input port of the link with the output port.
e.g, if we want to achieve the following link connection.
-------------------------------------------------------------------------
Source0 -> In0 | | Out0 ->
Source1 -> In1 |Funnel 0| Out1 ->
Source2 -> In2 | | Out2 ->
-------------------------------------------------------------------------
We can configure the "source" entries as the following device tree.
Funnel 0 {
out-ports {
port@0 {
reg = <0>
Out0: endpoint {
remote-endpoint = <... ...>;
source = <Source0>
}

}
port@1 {
reg = <1>
Out1: endpoint {
remote-endpoint = <... ...>;
source = <Source1>
}

}
port@2 {
reg = <0>
Out2: endpoint {
remote-endpoint = <... ...>;
source = <Source2>
}

}
}

in-ports {
port@0 {
reg = <0>
In0: endpoint {
remote-endpoint = <... ...>;
}

}
port@1 {
reg = <1>
Out1: endpoint {
remote-endpoint = <... ...>;
}

}
port@2 {
reg = <0>
Out2: endpoint {
remote-endpoint = <... ...>;
}

}
}
}

Best,
Tao