Funnel can support multi-port output in our hardware design. Since original
funnels only support a single output connection, the code needs to be
modified to support this new feature. The following is a typical topology
diagram of multi-port output of the funnels.
|---------| |---------| |---------| |---------| |---------|
| TPDM0 | | TPDM1 | | TPDM2 | | TPDM3 | | TPDM4 |
|---------| |---------| |---------| |---------| |---------|
| | | | |
| | | | |
| | | | |
|-----| |-----| |-----| |-----| |
| | | | |
| | | | |
[0]| |[1] [0]| |[1] |
\-------------/ \-------------/ \-------------/
\ FUNNEL0 / \ FUNNEL1 / \ FUNNEL2 /
----------- ----------- -----------
[0]| |[1] [0]| |[1] |
| |---------- | | |
| | | | |
|-------| | |------- | | |--------- |
| | | | |
| | | | |
[0]| |[1] |[2] |[3] |[4]
\ ---------------------------------------------------/
\ TPDA0 /
\ /
------------------------------------------------
For example, TPDM0 and TPDM1 are connected to the [0] and [1] input ports
of the funnel respectively, and output from the [0] and [1] output ports.
In this way, when data is output from the Funnel's output port, it needs
to know the source component corresponding to this output port. Our
solution is to add a property named "label" in the devicetree to mark the
source corresponding to the output port.
After introducing this new feature, another new problem also needs to be
solved. For example, TPDA driver will search for all the TPDMs on a input
port. In the topology diagram above, when TPDA searches for TPDM from the
input port[0], it will find TPDM0 and TPDM1. Our solution is to add a new
property named "qcom,tpda-input-port" to mark the input port number of the
TPDA in the devicetree.
Tao Zhang (4):
dt-bindings: arm: qcom,coresight-funnel: Add label for multi-ouput
coresight: Add support for multiple output ports on the funnel
dt-bindings: arm: qcom,coresight-tpdm: Mark tpda input port number
coresight-tpda: Add support multi-port input on TPDA
.../arm/arm,coresight-dynamic-funnel.yaml | 34 +++++++-
.../bindings/arm/qcom,coresight-tpdm.yaml | 8 ++
drivers/hwtracing/coresight/coresight-core.c | 81 ++++++++++++++++---
.../hwtracing/coresight/coresight-platform.c | 5 ++
drivers/hwtracing/coresight/coresight-tpda.c | 27 ++++++-
include/linux/coresight.h | 2 +
6 files changed, 139 insertions(+), 18 deletions(-)
--
2.17.1
Add new property "label" to label the source corresponding to the
output connection. When the funnel supports multi-output, this
property needs to be introduced to mark which source component a
certain output connection corresponds to.
Signed-off-by: Tao Zhang <[email protected]>
---
.../arm/arm,coresight-dynamic-funnel.yaml | 34 ++++++++++++++++---
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
index 44a1041cb0fc..cde62c286d29 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
@@ -66,13 +66,39 @@ properties:
$ref: /schemas/graph.yaml#/properties/port
out-ports:
- $ref: /schemas/graph.yaml#/properties/ports
- additionalProperties: false
-
+ type: object
properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
port:
+ type: object
+
+ patternProperties:
+ '^port(@[0-7])?$':
+ type: object
description: Output connection to CoreSight Trace bus
- $ref: /schemas/graph.yaml#/properties/port
+
+ patternProperties:
+ "^endpoint(@[0-9a-f]+)?$":
+ type: object
+ properties:
+ remote-endpoint:
+ description: |
+ phandle to an 'endpoint' subnode of a remote device node.
+ $ref: /schemas/types.yaml#/definitions/phandle
+ label:
+ description: Label the source corresponding to the output connection
+ $ref: /schemas/types.yaml#/definitions/string
+ oneOf:
+ - required:
+ - port
+ - required:
+ - "#address-cells"
+ - "#size-cells"
required:
- compatible
--
2.17.1
Since the funnel supports multi-port output scenario, multiple
TPDMs may be connected to one TPDA. Add a new property
"qcom,tpda-input-port" to mark the input port number of the TPDA
in the device tree, TPDA driver can find out the right TPDM it needs
to search according to this new mark property.
Signed-off-by: Tao Zhang <[email protected]>
---
.../devicetree/bindings/arm/qcom,coresight-tpdm.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
index 8eec07d9d454..383c0f5a658b 100644
--- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
@@ -77,6 +77,12 @@ properties:
minimum: 0
maximum: 32
+ qcom,tpda-input-port:
+ description:
+ Specifies the number of the input port on the corresponding TPDA.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+
clocks:
maxItems: 1
@@ -112,6 +118,7 @@ examples:
qcom,dsb-element-bits = <32>;
qcom,dsb-msrs-num = <16>;
+ qcom,tpda-input-port = <0>;
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
@@ -132,6 +139,7 @@ examples:
qcom,cmb-element-bits = <64>;
qcom,cmb-msrs-num = <32>;
+ qcom,tpda-input-port = <4>;
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
--
2.17.1
Since the funnel supports multi-port output scenarios, there may
be more than one TPDM connected to one TPDA input port. In this
way, when reading the element size of the TPDM, TPDA driver needs
to find the correct TPDM corresponding to the input port. When
TPDA finds a TPDM on an input port, it read the device tree of
the TPDM and finds the configured TPDA input port number. If it
is the same as the input port number passed into the function,
then it is the correct TPDM that needs to be found.
Signed-off-by: Tao Zhang <[email protected]>
---
drivers/hwtracing/coresight/coresight-tpda.c | 27 +++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 52b0201090fb..ba71e1ff18e3 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -84,6 +84,26 @@ static int tpdm_read_element_size(struct tpda_drvdata *drvdata,
return rc;
}
+/*
+ * Check if the parameter of the input port number in "tpda_get_element_size"
+ * is the same as the property of the TPDA input port number defined in the device
+ * tree.
+ * Return true if they are the same or the property is not read.
+ * Otherwise, return false.
+ */
+static bool is_tpda_inport_matched(struct coresight_device *csdev, u32 tpda_inport)
+{
+ int rc = -EINVAL;
+ u32 inport_nr;
+
+ rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent),
+ "qcom,tpda-input-port", &inport_nr);
+ if (!rc)
+ return (inport_nr == tpda_inport);
+
+ return true;
+}
+
/*
* Search and read element data size from the TPDM node in
* the devicetree. Each input port of TPDA is connected to
@@ -99,6 +119,10 @@ static int tpda_get_element_size(struct tpda_drvdata *drvdata,
int rc = 0;
int i;
struct coresight_device *in;
+ static u32 tpda_inport;
+
+ if (inport != -1)
+ tpda_inport = inport;
for (i = 0; i < csdev->pdata->nr_inconns; i++) {
in = csdev->pdata->in_conns[i]->src_dev;
@@ -110,7 +134,8 @@ static int tpda_get_element_size(struct tpda_drvdata *drvdata,
csdev->pdata->in_conns[i]->dest_port != inport)
continue;
- if (coresight_device_is_tpdm(in)) {
+ if (coresight_device_is_tpdm(in) &&
+ (is_tpda_inport_matched(in, tpda_inport))) {
if (drvdata->dsb_esize || drvdata->cmb_esize)
return -EEXIST;
rc = tpdm_read_element_size(drvdata, in);
--
2.17.1
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
port is selected according to the source in the trace path.
The source of the input port on funnels will be marked in the
device tree.
e.g.
tpdm@xxxxxxx {
... ... ... ...
};
funnel_XXX: funnel@xxxxxxx {
... ... ... ...
out-ports {
... ... ... ...
port@x {
... ... ... ...
label = "xxxxxxx.tpdm"; <-- To label the source
}; corresponding to the output
... ... ... ... connection "port@x". And this
}; is a hardware static connections.
... ... ... ... Here needs to refer to hardware
}; design.
Then driver will parse the source label marked in the device tree, and
save it to the coresight path. When the function needs to know the
source label, it could obtain it from coresight path parameter. Finally,
the output port knows which source it corresponds to, and it also knows
which input port it corresponds to.
Signed-off-by: Tao Zhang <[email protected]>
---
drivers/hwtracing/coresight/coresight-core.c | 81 ++++++++++++++++---
.../hwtracing/coresight/coresight-platform.c | 5 ++
include/linux/coresight.h | 2 +
3 files changed, 75 insertions(+), 13 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 5dde597403b3..b1b5e6d9ec7a 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -113,15 +113,63 @@ struct coresight_device *coresight_get_percpu_sink(int cpu)
}
EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
+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;
+}
+
+/**
+ * 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_label == NULL)
+ return ret;
+
+ source = coresight_get_source(path);
+ if (source == NULL)
+ return ret;
+
+ if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
+ conn->source_label))
+ ret = 0;
+ else
+ ret = -1;
+
+ return ret;
+}
+
static struct coresight_connection *
coresight_find_out_connection(struct coresight_device *src_dev,
- struct coresight_device *dest_dev)
+ struct coresight_device *dest_dev,
+ struct list_head *path)
{
int i;
struct coresight_connection *conn;
for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
conn = src_dev->pdata->out_conns[i];
+ if (coresight_source_filter(path, conn))
+ continue;
if (conn->dest_dev == dest_dev)
return conn;
}
@@ -312,7 +360,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;
@@ -321,8 +370,8 @@ static int coresight_enable_link(struct coresight_device *csdev,
if (!parent || !child)
return -EINVAL;
- inconn = coresight_find_out_connection(parent, csdev);
- outconn = coresight_find_out_connection(csdev, child);
+ inconn = coresight_find_out_connection(parent, csdev, path);
+ outconn = coresight_find_out_connection(csdev, child, path);
link_subtype = csdev->subtype.link_subtype;
if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && IS_ERR(inconn))
@@ -341,7 +390,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;
int link_subtype;
@@ -350,8 +400,8 @@ static void coresight_disable_link(struct coresight_device *csdev,
if (!parent || !child)
return;
- inconn = coresight_find_out_connection(parent, csdev);
- outconn = coresight_find_out_connection(csdev, child);
+ inconn = coresight_find_out_connection(parent, csdev, path);
+ outconn = coresight_find_out_connection(csdev, child, path);
link_subtype = csdev->subtype.link_subtype;
if (link_ops(csdev)->disable) {
@@ -507,7 +557,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;
@@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head *path, enum cs_mode mode,
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;
@@ -802,7 +852,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;
@@ -814,7 +865,7 @@ static int _coresight_build_path(struct coresight_device *csdev,
if (coresight_is_percpu_source(csdev) && coresight_is_percpu_sink(sink) &&
sink == per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev))) {
- if (_coresight_build_path(sink, sink, path) == 0) {
+ if (_coresight_build_path(sink, sink, path, source) == 0) {
found = true;
goto out;
}
@@ -825,8 +876,12 @@ static int _coresight_build_path(struct coresight_device *csdev,
struct coresight_device *child_dev;
child_dev = csdev->pdata->out_conns[i]->dest_dev;
+ if (csdev->pdata->out_conns[i]->source_label &&
+ !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
+ csdev->pdata->out_conns[i]->source_label))
+ continue;
if (child_dev &&
- _coresight_build_path(child_dev, sink, path) == 0) {
+ _coresight_build_path(child_dev, sink, path, source) == 0) {
found = true;
break;
}
@@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct device *dev,
struct fwnode_handle *rdev_fwnode;
struct coresight_connection conn = {};
struct coresight_connection *new_conn;
+ const char *label;
do {
/* Parse the local port details */
@@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct device *dev,
conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
conn.dest_port = rendpoint.port;
+ conn.source_label = NULL;
+ if (!of_property_read_string(ep, "label", &label))
+ conn.source_label = label;
+
new_conn = coresight_add_out_conn(dev, pdata, &conn);
if (IS_ERR_VALUE(new_conn)) {
fwnode_handle_put(conn.dest_fwnode);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index e8b6e388218c..a9c06ef9bbb2 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -167,6 +167,7 @@ struct coresight_desc {
* struct coresight_connection - representation of a single connection
* @src_port: a connection's output port number.
* @dest_port: destination's input port number @src_port is connected to.
+ * @source_label: source component's label.
* @dest_fwnode: destination component's fwnode handle.
* @dest_dev: a @coresight_device representation of the component
connected to @src_port. NULL until the device is created
@@ -195,6 +196,7 @@ struct coresight_desc {
struct coresight_connection {
int src_port;
int dest_port;
+ const char *source_label;
struct fwnode_handle *dest_fwnode;
struct coresight_device *dest_dev;
struct coresight_sysfs_link *link;
--
2.17.1
On Thu, Mar 21, 2024 at 04:32:04PM +0800, Tao Zhang wrote:
> Add new property "label" to label the source corresponding to the
> output connection. When the funnel supports multi-output, this
> property needs to be introduced to mark which source component a
> certain output connection corresponds to.
>
> Signed-off-by: Tao Zhang <[email protected]>
> ---
> .../arm/arm,coresight-dynamic-funnel.yaml | 34 ++++++++++++++++---
> 1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
> index 44a1041cb0fc..cde62c286d29 100644
> --- a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
> +++ b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
> @@ -66,13 +66,39 @@ properties:
> $ref: /schemas/graph.yaml#/properties/port
>
> out-ports:
> - $ref: /schemas/graph.yaml#/properties/ports
> - additionalProperties: false
> -
> + type: object
> properties:
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> port:
> + type: object
> +
> + patternProperties:
> + '^port(@[0-7])?$':
> + type: object
> description: Output connection to CoreSight Trace bus
> - $ref: /schemas/graph.yaml#/properties/port
Nope, now you have no constraints on port node properties. Please look
at how other bindings are done to add properties on endpoint node.
> +
> + patternProperties:
> + "^endpoint(@[0-9a-f]+)?$":
> + type: object
> + properties:
> + remote-endpoint:
> + description: |
> + phandle to an 'endpoint' subnode of a remote device node.
> + $ref: /schemas/types.yaml#/definitions/phandle
Don't need this.
> + label:
> + description: Label the source corresponding to the output connection
> + $ref: /schemas/types.yaml#/definitions/string
label already has a type.
As this node is an output, aren't you labeling what the destination is,
not the "source"?
Why can't you look at the remote connection to identify what it is?
> + oneOf:
> + - required:
> + - port
> + - required:
> + - "#address-cells"
> + - "#size-cells"
The common schema that you removed handles this.
Rob
On 21/03/2024 14:42, Rob Herring wrote:
> On Thu, Mar 21, 2024 at 04:32:04PM +0800, Tao Zhang wrote:
>> Add new property "label" to label the source corresponding to the
>> output connection. When the funnel supports multi-output, this
>> property needs to be introduced to mark which source component a
>> certain output connection corresponds to.
>>
>> Signed-off-by: Tao Zhang <[email protected]>
>> ---
>> .../arm/arm,coresight-dynamic-funnel.yaml | 34 ++++++++++++++++---
>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>> index 44a1041cb0fc..cde62c286d29 100644
>> --- a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>> +++ b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>> @@ -66,13 +66,39 @@ properties:
>> $ref: /schemas/graph.yaml#/properties/port
>>
>> out-ports:
>> - $ref: /schemas/graph.yaml#/properties/ports
>> - additionalProperties: false
>> -
>> + type: object
>> properties:
>> + "#address-cells":
>> + const: 1
>> +
>> + "#size-cells":
>> + const: 0
>> +
>> port:
>> + type: object
>> +
>> + patternProperties:
>> + '^port(@[0-7])?$':
>> + type: object
>> description: Output connection to CoreSight Trace bus
>> - $ref: /schemas/graph.yaml#/properties/port
>
> Nope, now you have no constraints on port node properties. Please look
> at how other bindings are done to add properties on endpoint node.
>
>> +
>> + patternProperties:
>> + "^endpoint(@[0-9a-f]+)?$":
>> + type: object
>> + properties:
>> + remote-endpoint:
>> + description: |
>> + phandle to an 'endpoint' subnode of a remote device node.
>> + $ref: /schemas/types.yaml#/definitions/phandle
>
> Don't need this.
>
>> + label:
>> + description: Label the source corresponding to the output connection
>> + $ref: /schemas/types.yaml#/definitions/string
>
> label already has a type.
>
> As this node is an output, aren't you labeling what the destination is,
> not the "source"?
>
> Why can't you look at the remote connection to identify what it is?
+1
Suzuki
>
>
>> + oneOf:
>> + - required:
>> + - port
>> + - required:
>> + - "#address-cells"
>> + - "#size-cells"
>
> The common schema that you removed handles this.
>
> Rob
On 21/03/2024 08:32, Tao Zhang wrote:
> 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
> port is selected according to the source in the trace path.
>
> The source of the input port on funnels will be marked in the
> device tree.
> e.g.
> tpdm@xxxxxxx {
> ... ... ... ...
> };
>
> funnel_XXX: funnel@xxxxxxx {
> ... ... ... ...
> out-ports {
> ... ... ... ...
> port@x {
> ... ... ... ...
> label = "xxxxxxx.tpdm"; <-- To label the source
> }; corresponding to the output
> ... ... ... ... connection "port@x". And this
> }; is a hardware static connections.
> ... ... ... ... Here needs to refer to hardware
> }; design.
>
> Then driver will parse the source label marked in the device tree, and
> save it to the coresight path. When the function needs to know the
> source label, it could obtain it from coresight path parameter. Finally,
> the output port knows which source it corresponds to, and it also knows
> which input port it corresponds to.
Why do we need labels ? We have connection information for all devices
(both in and out), so, why do we need this label to find a device ?
And also, I thought TPDM is a source device, why does a funnel output
port link to a source ?
Are these funnels programmable ? Or, are they static ? If they are
static, do these need to be described in the DT ? If they are simply
acting as a "LINK" (or HWFIFO ?)
Suzuki
>
> Signed-off-by: Tao Zhang <[email protected]>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 81 ++++++++++++++++---
> .../hwtracing/coresight/coresight-platform.c | 5 ++
> include/linux/coresight.h | 2 +
> 3 files changed, 75 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 5dde597403b3..b1b5e6d9ec7a 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -113,15 +113,63 @@ struct coresight_device *coresight_get_percpu_sink(int cpu)
> }
> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>
> +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;
> +}
> +
> +/**
> + * 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_label == NULL)
> + return ret;
> +
> + source = coresight_get_source(path);
> + if (source == NULL)
> + return ret;
> +
> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> + conn->source_label))
> + ret = 0;
> + else
> + ret = -1;
> +
> + return ret;
> +}
> +
> static struct coresight_connection *
> coresight_find_out_connection(struct coresight_device *src_dev,
> - struct coresight_device *dest_dev)
> + struct coresight_device *dest_dev,
> + struct list_head *path)
> {
> int i;
> struct coresight_connection *conn;
>
> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
> conn = src_dev->pdata->out_conns[i];
> + if (coresight_source_filter(path, conn))
> + continue;
> if (conn->dest_dev == dest_dev)
> return conn;
> }
> @@ -312,7 +360,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;
> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct coresight_device *csdev,
> if (!parent || !child)
> return -EINVAL;
>
> - inconn = coresight_find_out_connection(parent, csdev);
> - outconn = coresight_find_out_connection(csdev, child);
> + inconn = coresight_find_out_connection(parent, csdev, path);
> + outconn = coresight_find_out_connection(csdev, child, path);
> link_subtype = csdev->subtype.link_subtype;
>
> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && IS_ERR(inconn))
> @@ -341,7 +390,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;
> int link_subtype;
> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct coresight_device *csdev,
> if (!parent || !child)
> return;
>
> - inconn = coresight_find_out_connection(parent, csdev);
> - outconn = coresight_find_out_connection(csdev, child);
> + inconn = coresight_find_out_connection(parent, csdev, path);
> + outconn = coresight_find_out_connection(csdev, child, path);
> link_subtype = csdev->subtype.link_subtype;
>
> if (link_ops(csdev)->disable) {
> @@ -507,7 +557,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;
> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head *path, enum cs_mode mode,
> 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;
> @@ -802,7 +852,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;
> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct coresight_device *csdev,
>
> if (coresight_is_percpu_source(csdev) && coresight_is_percpu_sink(sink) &&
> sink == per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev))) {
> - if (_coresight_build_path(sink, sink, path) == 0) {
> + if (_coresight_build_path(sink, sink, path, source) == 0) {
> found = true;
> goto out;
> }
> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct coresight_device *csdev,
> struct coresight_device *child_dev;
>
> child_dev = csdev->pdata->out_conns[i]->dest_dev;
> + if (csdev->pdata->out_conns[i]->source_label &&
> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> + csdev->pdata->out_conns[i]->source_label))
> + continue;
> if (child_dev &&
> - _coresight_build_path(child_dev, sink, path) == 0) {
> + _coresight_build_path(child_dev, sink, path, source) == 0) {
> found = true;
> break;
> }
> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
> --- a/drivers/hwtracing/coresight/coresight-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct device *dev,
> struct fwnode_handle *rdev_fwnode;
> struct coresight_connection conn = {};
> struct coresight_connection *new_conn;
> + const char *label;
>
> do {
> /* Parse the local port details */
> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct device *dev,
> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
> conn.dest_port = rendpoint.port;
>
> + conn.source_label = NULL;
> + if (!of_property_read_string(ep, "label", &label))
> + conn.source_label = label;
> +
> new_conn = coresight_add_out_conn(dev, pdata, &conn);
> if (IS_ERR_VALUE(new_conn)) {
> fwnode_handle_put(conn.dest_fwnode);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index e8b6e388218c..a9c06ef9bbb2 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -167,6 +167,7 @@ struct coresight_desc {
> * struct coresight_connection - representation of a single connection
> * @src_port: a connection's output port number.
> * @dest_port: destination's input port number @src_port is connected to.
> + * @source_label: source component's label.
> * @dest_fwnode: destination component's fwnode handle.
> * @dest_dev: a @coresight_device representation of the component
> connected to @src_port. NULL until the device is created
> @@ -195,6 +196,7 @@ struct coresight_desc {
> struct coresight_connection {
> int src_port;
> int dest_port;
> + const char *source_label;
> struct fwnode_handle *dest_fwnode;
> struct coresight_device *dest_dev;
> struct coresight_sysfs_link *link;
On 3/21/2024 10:42 PM, Rob Herring wrote:
> On Thu, Mar 21, 2024 at 04:32:04PM +0800, Tao Zhang wrote:
>> Add new property "label" to label the source corresponding to the
>> output connection. When the funnel supports multi-output, this
>> property needs to be introduced to mark which source component a
>> certain output connection corresponds to.
>>
>> Signed-off-by: Tao Zhang <[email protected]>
>> ---
>> .../arm/arm,coresight-dynamic-funnel.yaml | 34 ++++++++++++++++---
>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>> index 44a1041cb0fc..cde62c286d29 100644
>> --- a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>> +++ b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>> @@ -66,13 +66,39 @@ properties:
>> $ref: /schemas/graph.yaml#/properties/port
>>
>> out-ports:
>> - $ref: /schemas/graph.yaml#/properties/ports
>> - additionalProperties: false
>> -
>> + type: object
>> properties:
>> + "#address-cells":
>> + const: 1
>> +
>> + "#size-cells":
>> + const: 0
>> +
>> port:
>> + type: object
>> +
>> + patternProperties:
>> + '^port(@[0-7])?$':
>> + type: object
>> description: Output connection to CoreSight Trace bus
>> - $ref: /schemas/graph.yaml#/properties/port
>
> Nope, now you have no constraints on port node properties. Please look
> at how other bindings are done to add properties on endpoint node.
>
Thanks for pointing this out, Rob. Shall we ref port-base and
endpoint-base then add new properties on endpoint? In this way, the
redundant code from port schema is not required.
>> +
>> + patternProperties:
>> + "^endpoint(@[0-9a-f]+)?$":
>> + type: object
>> + properties:
>> + remote-endpoint:
>> + description: |
>> + phandle to an 'endpoint' subnode of a remote device node.
>> + $ref: /schemas/types.yaml#/definitions/phandle
>
> Don't need this.
>
>> + label:
>> + description: Label the source corresponding to the output connection
>> + $ref: /schemas/types.yaml#/definitions/string
>
> label already has a type.
>
> As this node is an output, aren't you labeling what the destination is,
> not the "source"?
>
> Why can't you look at the remote connection to identify what it is?
>
This funnel can route data stream from different trace source to
different output ports. This lable property is added to describe which
source is routed to this output port.
For example, the graph is as below. Funnel3 routes trace data from TPDM0
to output[0] and output[0] of funnel3 is connected to input[0] of TPDA0.
While Funnels routes trace data from TPDM1 to output[1] which connects
to input[1] of TPDA0. Hope that clarifies this a little bit.
|---------| |---------| |---------| |---------| |---------|
| TPDM0 | | TPDM1 | | TPDM2 | | TPDM3 | | TPDM4 |
|---------| |---------| |---------| |---------| |---------|
| | | | |
| | | | |
| | | | |
|-----| |-----| |-----| |-----| |
| | | | |
| | | | |
[0]| |[1] [0]| |[1] |
\-------------/ \-------------/ \------------/
\ FUNNEL0 / \ FUNNEL1 / \ FUNNEL2 /
----------- ----------- -----------
| | |
\-------------/ \-------------/ |
\ FUNNEL3 / \ FUNNEL4 / |
----------- ----------- |
| | | |
[0]| |[1] [0]| |[1] |
| |---------- | | |
| | | | |
|-------| | |------- | | |--------- |
| | | | |
| | | | |
[0]| |[1] |[2] |[3] |[4]
\ ---------------------------------------------------/
\ TPDA0 /
\ /
------------------------------------------------
>
>> + oneOf:
>> + - required:
>> + - port
>> + - required:
>> + - "#address-cells"
>> + - "#size-cells"
>
> The common schema that you removed handles this.
>
> Rob
--
Thanks,
Tingwei
On 22/03/2024 07:02, Tingwei Zhang wrote:
> On 3/21/2024 10:42 PM, Rob Herring wrote:
>> On Thu, Mar 21, 2024 at 04:32:04PM +0800, Tao Zhang wrote:
>>> Add new property "label" to label the source corresponding to the
>>> output connection. When the funnel supports multi-output, this
>>> property needs to be introduced to mark which source component a
>>> certain output connection corresponds to.
>>>
>>> Signed-off-by: Tao Zhang <[email protected]>
>>> ---
>>> .../arm/arm,coresight-dynamic-funnel.yaml | 34 ++++++++++++++++---
>>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>>> index 44a1041cb0fc..cde62c286d29 100644
>>> ---
>>> a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>>> +++
>>> b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>>> @@ -66,13 +66,39 @@ properties:
>>> $ref: /schemas/graph.yaml#/properties/port
>>> out-ports:
>>> - $ref: /schemas/graph.yaml#/properties/ports
>>> - additionalProperties: false
>>> -
>>> + type: object
>>> properties:
>>> + "#address-cells":
>>> + const: 1
>>> +
>>> + "#size-cells":
>>> + const: 0
>>> +
>>> port:
>>> + type: object
>>> +
>>> + patternProperties:
>>> + '^port(@[0-7])?$':
>>> + type: object
>>> description: Output connection to CoreSight Trace bus
>>> - $ref: /schemas/graph.yaml#/properties/port
>>
>> Nope, now you have no constraints on port node properties. Please look
>> at how other bindings are done to add properties on endpoint node.
>>
> Thanks for pointing this out, Rob. Shall we ref port-base and
> endpoint-base then add new properties on endpoint? In this way, the
> redundant code from port schema is not required.
>>> +
>>> + patternProperties:
>>> + "^endpoint(@[0-9a-f]+)?$":
>>> + type: object
>>> + properties:
>>> + remote-endpoint:
>>> + description: |
>>> + phandle to an 'endpoint' subnode of a remote
>>> device node.
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>
>> Don't need this.
>>
>>> + label:
>>> + description: Label the source corresponding to the
>>> output connection
>>> + $ref: /schemas/types.yaml#/definitions/string
>>
>> label already has a type.
>>
>> As this node is an output, aren't you labeling what the destination is,
>> not the "source"?
>>
>> Why can't you look at the remote connection to identify what it is?
>>
> This funnel can route data stream from different trace source to
> different output ports. This lable property is added to describe which
> source is routed to this output port.
>
> For example, the graph is as below. Funnel3 routes trace data from TPDM0
> to output[0] and output[0] of funnel3 is connected to input[0] of TPDA0.
Funnel3 and Funnel4 are really Replicators ! How are they Funnels ?
Again, my question still stands. Are Funnel(Replicator-renamed)3/4 and
Funnel 0/1/2 programmable ?
Suzuki
> While Funnels routes trace data from TPDM1 to output[1] which connects
> to input[1] of TPDA0. Hope that clarifies this a little bit.
>
> |---------| |---------| |---------| |---------| |---------|
> | TPDM0 | | TPDM1 | | TPDM2 | | TPDM3 | | TPDM4 |
> |---------| |---------| |---------| |---------| |---------|
> | | | | |
> | | | | |
> | | | | |
> |-----| |-----| |-----| |-----| |
> | | | | |
> | | | | |
> [0]| |[1] [0]| |[1] |
> \-------------/ \-------------/ \------------/
> \ FUNNEL0 / \ FUNNEL1 / \ FUNNEL2 /
> ----------- ----------- -----------
> | | |
> \-------------/ \-------------/ |
> \ FUNNEL3 / \ FUNNEL4 / |
> ----------- ----------- |
> | | | |
> [0]| |[1] [0]| |[1] |
> | |---------- | | |
> | | | | |
> |-------| | |------- | | |--------- |
> | | | | |
> | | | | |
> [0]| |[1] |[2] |[3] |[4]
> \ ---------------------------------------------------/
> \ TPDA0 /
> \ /
> ------------------------------------------------
>
>>
>>> + oneOf:
>>> + - required:
>>> + - port
>>> + - required:
>>> + - "#address-cells"
>>> + - "#size-cells"
>>
>> The common schema that you removed handles this.
>>
>> Rob
>
On 3/22/2024 5:42 PM, Suzuki K Poulose wrote:
> On 22/03/2024 07:02, Tingwei Zhang wrote:
>> On 3/21/2024 10:42 PM, Rob Herring wrote:
>>> On Thu, Mar 21, 2024 at 04:32:04PM +0800, Tao Zhang wrote:
>>>> Add new property "label" to label the source corresponding to the
>>>> output connection. When the funnel supports multi-output, this
>>>> property needs to be introduced to mark which source component a
>>>> certain output connection corresponds to.
>>>>
>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>> ---
>>>> .../arm/arm,coresight-dynamic-funnel.yaml | 34
>>>> ++++++++++++++++---
>>>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>>>> index 44a1041cb0fc..cde62c286d29 100644
>>>> ---
>>>> a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>>>> +++
>>>> b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
>>>> @@ -66,13 +66,39 @@ properties:
>>>> $ref: /schemas/graph.yaml#/properties/port
>>>> out-ports:
>>>> - $ref: /schemas/graph.yaml#/properties/ports
>>>> - additionalProperties: false
>>>> -
>>>> + type: object
>>>> properties:
>>>> + "#address-cells":
>>>> + const: 1
>>>> +
>>>> + "#size-cells":
>>>> + const: 0
>>>> +
>>>> port:
>>>> + type: object
>>>> +
>>>> + patternProperties:
>>>> + '^port(@[0-7])?$':
>>>> + type: object
>>>> description: Output connection to CoreSight Trace bus
>>>> - $ref: /schemas/graph.yaml#/properties/port
>>>
>>> Nope, now you have no constraints on port node properties. Please look
>>> at how other bindings are done to add properties on endpoint node.
>>>
>> Thanks for pointing this out, Rob. Shall we ref port-base and
>> endpoint-base then add new properties on endpoint? In this way, the
>> redundant code from port schema is not required.
>>>> +
>>>> + patternProperties:
>>>> + "^endpoint(@[0-9a-f]+)?$":
>>>> + type: object
>>>> + properties:
>>>> + remote-endpoint:
>>>> + description: |
>>>> + phandle to an 'endpoint' subnode of a remote
>>>> device node.
>>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>>
>>> Don't need this.
>>>
>>>> + label:
>>>> + description: Label the source corresponding to the
>>>> output connection
>>>> + $ref: /schemas/types.yaml#/definitions/string
>>>
>>> label already has a type.
>>>
>>> As this node is an output, aren't you labeling what the destination is,
>>> not the "source"?
>>>
>>> Why can't you look at the remote connection to identify what it is?
>>>
>> This funnel can route data stream from different trace source to
>> different output ports. This lable property is added to describe which
>> source is routed to this output port.
>>
>> For example, the graph is as below. Funnel3 routes trace data from
>> TPDM0 to output[0] and output[0] of funnel3 is connected to input[0]
>> of TPDA0.
>
> Funnel3 and Funnel4 are really Replicators ! How are they Funnels ?
> Again, my question still stands. Are Funnel(Replicator-renamed)3/4 and
> Funnel 0/1/2 programmable ?
Sorry for oversimplied the topology. Funnel3 and Funnel4 have multiple
input ports instead of just one input port. It can have multiple input
ports and multiple output ports. Unlike replicator, it won't replicate
same data trace to all the outputs.
Funnel3/funnel4 has same programing capability like standard coresight
funnel. It can enable input ports as requested. It can not be programed
to route which source to which output ports. Hardware staticlly defined
which source is routed to which output.
>
> Suzuki
>
>
>> While Funnels routes trace data from TPDM1 to output[1] which connects
>> to input[1] of TPDA0. Hope that clarifies this a little bit.
>>
>> |---------| |---------| |---------| |---------| |---------|
>> | TPDM0 | | TPDM1 | | TPDM2 | | TPDM3 | | TPDM4 |
>> |---------| |---------| |---------| |---------| |---------|
>> | | | | |
>> | | | | |
>> | | | | |
>> |-----| |-----| |-----| |-----| |
>> | | | | |
>> | | | | |
>> [0]| |[1] [0]| |[1] |
>> \-------------/ \-------------/ \------------/
>> \ FUNNEL0 / \ FUNNEL1 / \ FUNNEL2 /
>> ----------- ----------- -----------
>> | | |
>> \-------------/ \-------------/ |
>> \ FUNNEL3 / \ FUNNEL4 / |
>> ----------- ----------- |
>> | | | |
>> [0]| |[1] [0]| |[1] |
>> | |---------- | | |
>> | | | | |
>> |-------| | |------- | | |--------- |
>> | | | | |
>> | | | | |
>> [0]| |[1] |[2] |[3] |[4]
>> \ ---------------------------------------------------/
>> \ TPDA0 /
>> \ /
>> ------------------------------------------------
>>
>>>
>>>> + oneOf:
>>>> + - required:
>>>> + - port
>>>> + - required:
>>>> + - "#address-cells"
>>>> + - "#size-cells"
>>>
>>> The common schema that you removed handles this.
>>>
>>> Rob
>>
>
--
Thanks,
Tingwei
On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
> On 21/03/2024 08:32, Tao Zhang wrote:
>> 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
>> port is selected according to the source in the trace path.
>>
>> The source of the input port on funnels will be marked in the
>> device tree.
>> e.g.
>> tpdm@xxxxxxx {
>> ... ... ... ...
>> };
>>
>> funnel_XXX: funnel@xxxxxxx {
>> ... ... ... ...
>> out-ports {
>> ... ... ... ...
>> port@x {
>> ... ... ... ...
>> label = "xxxxxxx.tpdm"; <-- To label the source
>> }; corresponding to the output
>> ... ... ... ... connection "port@x". And this
>> }; is a hardware static connections.
>> ... ... ... ... Here needs to refer to hardware
>> }; design.
>>
>> Then driver will parse the source label marked in the device tree, and
>> save it to the coresight path. When the function needs to know the
>> source label, it could obtain it from coresight path parameter. Finally,
>> the output port knows which source it corresponds to, and it also knows
>> which input port it corresponds to.
>
> Why do we need labels ? We have connection information for all devices
> (both in and out), so, why do we need this label to find a device ?
Because our funnel's design has multi-output ports, the data stream will not
know which output port should pass in building the data trace path. This
source
label can make the data stream find the right output port to go.
>
> And also, I thought TPDM is a source device, why does a funnel output
> port link to a source ?
No, this label doesn't mean this funnel output port link to a source, it
just let
the output port know its data source.
>
> Are these funnels programmable ? Or, are they static ? If they are
> static, do these need to be described in the DT ? If they are simply
> acting as a "LINK" (or HWFIFO ?)
These funnels are static, and we will add the "label" to the DT to
describe the
multi-output ports for these funnels.
"If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
what's the meaning
of this. Could you describe it in detail?
Best,
Tao
>
> Suzuki
>
>>
>> Signed-off-by: Tao Zhang <[email protected]>
>> ---
>> drivers/hwtracing/coresight/coresight-core.c | 81 ++++++++++++++++---
>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>> include/linux/coresight.h | 2 +
>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>> b/drivers/hwtracing/coresight/coresight-core.c
>> index 5dde597403b3..b1b5e6d9ec7a 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -113,15 +113,63 @@ struct coresight_device
>> *coresight_get_percpu_sink(int cpu)
>> }
>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>> +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;
>> +}
>> +
>> +/**
>> + * 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_label == NULL)
>> + return ret;
>> +
>> + source = coresight_get_source(path);
>> + if (source == NULL)
>> + return ret;
>> +
>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>> + conn->source_label))
>> + ret = 0;
>> + else
>> + ret = -1;
>> +
>> + return ret;
>> +}
>> +
>> static struct coresight_connection *
>> coresight_find_out_connection(struct coresight_device *src_dev,
>> - struct coresight_device *dest_dev)
>> + struct coresight_device *dest_dev,
>> + struct list_head *path)
>> {
>> int i;
>> struct coresight_connection *conn;
>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>> conn = src_dev->pdata->out_conns[i];
>> + if (coresight_source_filter(path, conn))
>> + continue;
>> if (conn->dest_dev == dest_dev)
>> return conn;
>> }
>> @@ -312,7 +360,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;
>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>> coresight_device *csdev,
>> if (!parent || !child)
>> return -EINVAL;
>> - inconn = coresight_find_out_connection(parent, csdev);
>> - outconn = coresight_find_out_connection(csdev, child);
>> + inconn = coresight_find_out_connection(parent, csdev, path);
>> + outconn = coresight_find_out_connection(csdev, child, path);
>> link_subtype = csdev->subtype.link_subtype;
>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>> IS_ERR(inconn))
>> @@ -341,7 +390,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;
>> int link_subtype;
>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>> coresight_device *csdev,
>> if (!parent || !child)
>> return;
>> - inconn = coresight_find_out_connection(parent, csdev);
>> - outconn = coresight_find_out_connection(csdev, child);
>> + inconn = coresight_find_out_connection(parent, csdev, path);
>> + outconn = coresight_find_out_connection(csdev, child, path);
>> link_subtype = csdev->subtype.link_subtype;
>> if (link_ops(csdev)->disable) {
>> @@ -507,7 +557,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;
>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head *path,
>> enum cs_mode mode,
>> 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;
>> @@ -802,7 +852,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;
>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>> coresight_device *csdev,
>> if (coresight_is_percpu_source(csdev) &&
>> coresight_is_percpu_sink(sink) &&
>> sink == per_cpu(csdev_sink,
>> source_ops(csdev)->cpu_id(csdev))) {
>> - if (_coresight_build_path(sink, sink, path) == 0) {
>> + if (_coresight_build_path(sink, sink, path, source) == 0) {
>> found = true;
>> goto out;
>> }
>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>> coresight_device *csdev,
>> struct coresight_device *child_dev;
>> child_dev = csdev->pdata->out_conns[i]->dest_dev;
>> + if (csdev->pdata->out_conns[i]->source_label &&
>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>> + csdev->pdata->out_conns[i]->source_label))
>> + continue;
>> if (child_dev &&
>> - _coresight_build_path(child_dev, sink, path) == 0) {
>> + _coresight_build_path(child_dev, sink, path, source) ==
>> 0) {
>> found = true;
>> break;
>> }
>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
>> device *dev,
>> struct fwnode_handle *rdev_fwnode;
>> struct coresight_connection conn = {};
>> struct coresight_connection *new_conn;
>> + const char *label;
>> do {
>> /* Parse the local port details */
>> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
>> device *dev,
>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>> conn.dest_port = rendpoint.port;
>> + conn.source_label = NULL;
>> + if (!of_property_read_string(ep, "label", &label))
>> + conn.source_label = label;
>> +
>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
>> if (IS_ERR_VALUE(new_conn)) {
>> fwnode_handle_put(conn.dest_fwnode);
>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>> index e8b6e388218c..a9c06ef9bbb2 100644
>> --- a/include/linux/coresight.h
>> +++ b/include/linux/coresight.h
>> @@ -167,6 +167,7 @@ struct coresight_desc {
>> * struct coresight_connection - representation of a single connection
>> * @src_port: a connection's output port number.
>> * @dest_port: destination's input port number @src_port is
>> connected to.
>> + * @source_label: source component's label.
>> * @dest_fwnode: destination component's fwnode handle.
>> * @dest_dev: a @coresight_device representation of the component
>> connected to @src_port. NULL until the device is created
>> @@ -195,6 +196,7 @@ struct coresight_desc {
>> struct coresight_connection {
>> int src_port;
>> int dest_port;
>> + const char *source_label;
>> struct fwnode_handle *dest_fwnode;
>> struct coresight_device *dest_dev;
>> struct coresight_sysfs_link *link;
>
Hi Tao,
Using a DT label in this way is not how connections should be
described in device tree, The association needs to be between the
input and output ports of the component not and extra link back to the
source.
If your "funnel" has multiple input and output ports, then it is no
longer a funnel. As you describe it - the input ports are statically
associated with an output port, so the device is effectively a set of
individual funnels.
Given that the association between the inputs and outputs is known at
time of compilation - then these can be inferred from the components
compatible string and handled internally during driver probe.
This component should therefore really have its own driver, and not be
described as a standard funnel. In this way you can handle the input
and output associations within the driver and create a path in the
normal way, avoiding the need for large changes to the coresight API.
Having a specific driver will allow creation of sets of associated
input and output connections.
Regards
Mike
On Fri, 29 Mar 2024 at 09:27, Tao Zhang <[email protected]> wrote:
>
>
> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
> > On 21/03/2024 08:32, Tao Zhang wrote:
> >> 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
> >> port is selected according to the source in the trace path.
> >>
> >> The source of the input port on funnels will be marked in the
> >> device tree.
> >> e.g.
> >> tpdm@xxxxxxx {
> >> ... ... ... ...
> >> };
> >>
> >> funnel_XXX: funnel@xxxxxxx {
> >> ... ... ... ...
> >> out-ports {
> >> ... ... ... ...
> >> port@x {
> >> ... ... ... ...
> >> label = "xxxxxxx.tpdm"; <-- To label the source
> >> }; corresponding to the output
> >> ... ... ... ... connection "port@x". And this
> >> }; is a hardware static connections.
> >> ... ... ... ... Here needs to refer to hardware
> >> }; design.
> >>
> >> Then driver will parse the source label marked in the device tree, and
> >> save it to the coresight path. When the function needs to know the
> >> source label, it could obtain it from coresight path parameter. Finally,
> >> the output port knows which source it corresponds to, and it also knows
> >> which input port it corresponds to.
> >
> > Why do we need labels ? We have connection information for all devices
> > (both in and out), so, why do we need this label to find a device ?
>
> Because our funnel's design has multi-output ports, the data stream will not
>
> know which output port should pass in building the data trace path. This
> source
>
> label can make the data stream find the right output port to go.
>
> >
> > And also, I thought TPDM is a source device, why does a funnel output
> > port link to a source ?
>
> No, this label doesn't mean this funnel output port link to a source, it
> just let
>
> the output port know its data source.
>
> >
> > Are these funnels programmable ? Or, are they static ? If they are
> > static, do these need to be described in the DT ? If they are simply
> > acting as a "LINK" (or HWFIFO ?)
>
> These funnels are static, and we will add the "label" to the DT to
> describe the
>
> multi-output ports for these funnels.
>
> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
> what's the meaning
>
> of this. Could you describe it in detail?
>
>
> Best,
>
> Tao
>
> >
> > Suzuki
> >
> >>
> >> Signed-off-by: Tao Zhang <[email protected]>
> >> ---
> >> drivers/hwtracing/coresight/coresight-core.c | 81 ++++++++++++++++---
> >> .../hwtracing/coresight/coresight-platform.c | 5 ++
> >> include/linux/coresight.h | 2 +
> >> 3 files changed, 75 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-core.c
> >> b/drivers/hwtracing/coresight/coresight-core.c
> >> index 5dde597403b3..b1b5e6d9ec7a 100644
> >> --- a/drivers/hwtracing/coresight/coresight-core.c
> >> +++ b/drivers/hwtracing/coresight/coresight-core.c
> >> @@ -113,15 +113,63 @@ struct coresight_device
> >> *coresight_get_percpu_sink(int cpu)
> >> }
> >> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
> >> +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;
> >> +}
> >> +
> >> +/**
> >> + * 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_label == NULL)
> >> + return ret;
> >> +
> >> + source = coresight_get_source(path);
> >> + if (source == NULL)
> >> + return ret;
> >> +
> >> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >> + conn->source_label))
> >> + ret = 0;
> >> + else
> >> + ret = -1;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> static struct coresight_connection *
> >> coresight_find_out_connection(struct coresight_device *src_dev,
> >> - struct coresight_device *dest_dev)
> >> + struct coresight_device *dest_dev,
> >> + struct list_head *path)
> >> {
> >> int i;
> >> struct coresight_connection *conn;
> >> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
> >> conn = src_dev->pdata->out_conns[i];
> >> + if (coresight_source_filter(path, conn))
> >> + continue;
> >> if (conn->dest_dev == dest_dev)
> >> return conn;
> >> }
> >> @@ -312,7 +360,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;
> >> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
> >> coresight_device *csdev,
> >> if (!parent || !child)
> >> return -EINVAL;
> >> - inconn = coresight_find_out_connection(parent, csdev);
> >> - outconn = coresight_find_out_connection(csdev, child);
> >> + inconn = coresight_find_out_connection(parent, csdev, path);
> >> + outconn = coresight_find_out_connection(csdev, child, path);
> >> link_subtype = csdev->subtype.link_subtype;
> >> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
> >> IS_ERR(inconn))
> >> @@ -341,7 +390,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;
> >> int link_subtype;
> >> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
> >> coresight_device *csdev,
> >> if (!parent || !child)
> >> return;
> >> - inconn = coresight_find_out_connection(parent, csdev);
> >> - outconn = coresight_find_out_connection(csdev, child);
> >> + inconn = coresight_find_out_connection(parent, csdev, path);
> >> + outconn = coresight_find_out_connection(csdev, child, path);
> >> link_subtype = csdev->subtype.link_subtype;
> >> if (link_ops(csdev)->disable) {
> >> @@ -507,7 +557,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;
> >> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head *path,
> >> enum cs_mode mode,
> >> 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;
> >> @@ -802,7 +852,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;
> >> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
> >> coresight_device *csdev,
> >> if (coresight_is_percpu_source(csdev) &&
> >> coresight_is_percpu_sink(sink) &&
> >> sink == per_cpu(csdev_sink,
> >> source_ops(csdev)->cpu_id(csdev))) {
> >> - if (_coresight_build_path(sink, sink, path) == 0) {
> >> + if (_coresight_build_path(sink, sink, path, source) == 0) {
> >> found = true;
> >> goto out;
> >> }
> >> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
> >> coresight_device *csdev,
> >> struct coresight_device *child_dev;
> >> child_dev = csdev->pdata->out_conns[i]->dest_dev;
> >> + if (csdev->pdata->out_conns[i]->source_label &&
> >> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >> + csdev->pdata->out_conns[i]->source_label))
> >> + continue;
> >> if (child_dev &&
> >> - _coresight_build_path(child_dev, sink, path) == 0) {
> >> + _coresight_build_path(child_dev, sink, path, source) ==
> >> 0) {
> >> found = true;
> >> break;
> >> }
> >> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
> >> --- a/drivers/hwtracing/coresight/coresight-platform.c
> >> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> >> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
> >> device *dev,
> >> struct fwnode_handle *rdev_fwnode;
> >> struct coresight_connection conn = {};
> >> struct coresight_connection *new_conn;
> >> + const char *label;
> >> do {
> >> /* Parse the local port details */
> >> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
> >> device *dev,
> >> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
> >> conn.dest_port = rendpoint.port;
> >> + conn.source_label = NULL;
> >> + if (!of_property_read_string(ep, "label", &label))
> >> + conn.source_label = label;
> >> +
> >> new_conn = coresight_add_out_conn(dev, pdata, &conn);
> >> if (IS_ERR_VALUE(new_conn)) {
> >> fwnode_handle_put(conn.dest_fwnode);
> >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> >> index e8b6e388218c..a9c06ef9bbb2 100644
> >> --- a/include/linux/coresight.h
> >> +++ b/include/linux/coresight.h
> >> @@ -167,6 +167,7 @@ struct coresight_desc {
> >> * struct coresight_connection - representation of a single connection
> >> * @src_port: a connection's output port number.
> >> * @dest_port: destination's input port number @src_port is
> >> connected to.
> >> + * @source_label: source component's label.
> >> * @dest_fwnode: destination component's fwnode handle.
> >> * @dest_dev: a @coresight_device representation of the component
> >> connected to @src_port. NULL until the device is created
> >> @@ -195,6 +196,7 @@ struct coresight_desc {
> >> struct coresight_connection {
> >> int src_port;
> >> int dest_port;
> >> + const char *source_label;
> >> struct fwnode_handle *dest_fwnode;
> >> struct coresight_device *dest_dev;
> >> struct coresight_sysfs_link *link;
> >
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Hi
On 29/03/2024 09:27, Tao Zhang wrote:
>
> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
>> On 21/03/2024 08:32, Tao Zhang wrote:
>>> 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
>>> port is selected according to the source in the trace path.
>>>
>>> The source of the input port on funnels will be marked in the
>>> device tree.
>>> e.g.
>>> tpdm@xxxxxxx {
>>> ... ... ... ...
>>> };
>>>
>>> funnel_XXX: funnel@xxxxxxx {
>>> ... ... ... ...
>>> out-ports {
>>> ... ... ... ...
>>> port@x {
>>> ... ... ... ...
>>> label = "xxxxxxx.tpdm"; <-- To label the source
>>> }; corresponding to the output
>>> ... ... ... ... connection "port@x". And this
>>> }; is a hardware static connections.
>>> ... ... ... ... Here needs to refer to hardware
>>> }; design.
>>>
>>> Then driver will parse the source label marked in the device tree, and
>>> save it to the coresight path. When the function needs to know the
>>> source label, it could obtain it from coresight path parameter. Finally,
>>> the output port knows which source it corresponds to, and it also knows
>>> which input port it corresponds to.
>>
>> Why do we need labels ? We have connection information for all devices
>> (both in and out), so, why do we need this label to find a device ?
>
> Because our funnel's design has multi-output ports, the data stream will
> not
>
> know which output port should pass in building the data trace path. This
> source
>
> label can make the data stream find the right output port to go.
>
>>
>> And also, I thought TPDM is a source device, why does a funnel output
>> port link to a source ?
>
> No, this label doesn't mean this funnel output port link to a source, it
> just let
>
> the output port know its data source.
>
>>
>> Are these funnels programmable ? Or, are they static ? If they are
>> static, do these need to be described in the DT ? If they are simply
>> acting as a "LINK" (or HWFIFO ?)
>
> These funnels are static, and we will add the "label" to the DT to
> describe the
>
> multi-output ports for these funnels.
I think there is still a bit of confusion. By "Dynamic" I mean,
the "dynamic funnel" (explicit port enablement via MMIO) vs "static
funnel" (no programming, always ON).
So, coming to your example, do we need to "explicitly" enable trace flow
for an "input" and/or an "output" port in your "funnel" ?
>
> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
> what's the meaning
i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is acting
like a cache for easing ATB data load, by providing h/w buffering.
(In your case, it may not be providing any buffering, it doesn't matter
either way, as it is not visible to the driver).
Suzuki
>
> of this. Could you describe it in detail?
>
>
> Best,
>
> Tao
>
>>
>> Suzuki
>>
>>>
>>> Signed-off-by: Tao Zhang <[email protected]>
>>> ---
>>> drivers/hwtracing/coresight/coresight-core.c | 81 ++++++++++++++++---
>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>>> include/linux/coresight.h | 2 +
>>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>> b/drivers/hwtracing/coresight/coresight-core.c
>>> index 5dde597403b3..b1b5e6d9ec7a 100644
>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>> @@ -113,15 +113,63 @@ struct coresight_device
>>> *coresight_get_percpu_sink(int cpu)
>>> }
>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>> +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;
>>> +}
>>> +
>>> +/**
>>> + * 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_label == NULL)
>>> + return ret;
>>> +
>>> + source = coresight_get_source(path);
>>> + if (source == NULL)
>>> + return ret;
>>> +
>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>> + conn->source_label))
>>> + ret = 0;
>>> + else
>>> + ret = -1;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> static struct coresight_connection *
>>> coresight_find_out_connection(struct coresight_device *src_dev,
>>> - struct coresight_device *dest_dev)
>>> + struct coresight_device *dest_dev,
>>> + struct list_head *path)
>>> {
>>> int i;
>>> struct coresight_connection *conn;
>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>> conn = src_dev->pdata->out_conns[i];
>>> + if (coresight_source_filter(path, conn))
>>> + continue;
>>> if (conn->dest_dev == dest_dev)
>>> return conn;
>>> }
>>> @@ -312,7 +360,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;
>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>>> coresight_device *csdev,
>>> if (!parent || !child)
>>> return -EINVAL;
>>> - inconn = coresight_find_out_connection(parent, csdev);
>>> - outconn = coresight_find_out_connection(csdev, child);
>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>> link_subtype = csdev->subtype.link_subtype;
>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>> IS_ERR(inconn))
>>> @@ -341,7 +390,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;
>>> int link_subtype;
>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>>> coresight_device *csdev,
>>> if (!parent || !child)
>>> return;
>>> - inconn = coresight_find_out_connection(parent, csdev);
>>> - outconn = coresight_find_out_connection(csdev, child);
>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>> link_subtype = csdev->subtype.link_subtype;
>>> if (link_ops(csdev)->disable) {
>>> @@ -507,7 +557,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;
>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head *path,
>>> enum cs_mode mode,
>>> 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;
>>> @@ -802,7 +852,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;
>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>>> coresight_device *csdev,
>>> if (coresight_is_percpu_source(csdev) &&
>>> coresight_is_percpu_sink(sink) &&
>>> sink == per_cpu(csdev_sink,
>>> source_ops(csdev)->cpu_id(csdev))) {
>>> - if (_coresight_build_path(sink, sink, path) == 0) {
>>> + if (_coresight_build_path(sink, sink, path, source) == 0) {
>>> found = true;
>>> goto out;
>>> }
>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>>> coresight_device *csdev,
>>> struct coresight_device *child_dev;
>>> child_dev = csdev->pdata->out_conns[i]->dest_dev;
>>> + if (csdev->pdata->out_conns[i]->source_label &&
>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>> + csdev->pdata->out_conns[i]->source_label))
>>> + continue;
>>> if (child_dev &&
>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>> + _coresight_build_path(child_dev, sink, path, source) ==
>>> 0) {
>>> found = true;
>>> break;
>>> }
>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
>>> device *dev,
>>> struct fwnode_handle *rdev_fwnode;
>>> struct coresight_connection conn = {};
>>> struct coresight_connection *new_conn;
>>> + const char *label;
>>> do {
>>> /* Parse the local port details */
>>> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
>>> device *dev,
>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>> conn.dest_port = rendpoint.port;
>>> + conn.source_label = NULL;
>>> + if (!of_property_read_string(ep, "label", &label))
>>> + conn.source_label = label;
>>> +
>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
>>> if (IS_ERR_VALUE(new_conn)) {
>>> fwnode_handle_put(conn.dest_fwnode);
>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>> index e8b6e388218c..a9c06ef9bbb2 100644
>>> --- a/include/linux/coresight.h
>>> +++ b/include/linux/coresight.h
>>> @@ -167,6 +167,7 @@ struct coresight_desc {
>>> * struct coresight_connection - representation of a single connection
>>> * @src_port: a connection's output port number.
>>> * @dest_port: destination's input port number @src_port is
>>> connected to.
>>> + * @source_label: source component's label.
>>> * @dest_fwnode: destination component's fwnode handle.
>>> * @dest_dev: a @coresight_device representation of the component
>>> connected to @src_port. NULL until the device is created
>>> @@ -195,6 +196,7 @@ struct coresight_desc {
>>> struct coresight_connection {
>>> int src_port;
>>> int dest_port;
>>> + const char *source_label;
>>> struct fwnode_handle *dest_fwnode;
>>> struct coresight_device *dest_dev;
>>> struct coresight_sysfs_link *link;
>>
On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
> Hi
>
> On 29/03/2024 09:27, Tao Zhang wrote:
>>
>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
>>> On 21/03/2024 08:32, Tao Zhang wrote:
>>>> 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
>>>> port is selected according to the source in the trace path.
>>>>
>>>> The source of the input port on funnels will be marked in the
>>>> device tree.
>>>> e.g.
>>>> tpdm@xxxxxxx {
>>>> ... ... ... ...
>>>> };
>>>>
>>>> funnel_XXX: funnel@xxxxxxx {
>>>> ... ... ... ...
>>>> out-ports {
>>>> ... ... ... ...
>>>> port@x {
>>>> ... ... ... ...
>>>> label = "xxxxxxx.tpdm"; <-- To label the source
>>>> }; corresponding to the output
>>>> ... ... ... ... connection "port@x". And this
>>>> }; is a hardware static
>>>> connections.
>>>> ... ... ... ... Here needs to refer to hardware
>>>> }; design.
>>>>
>>>> Then driver will parse the source label marked in the device tree, and
>>>> save it to the coresight path. When the function needs to know the
>>>> source label, it could obtain it from coresight path parameter.
>>>> Finally,
>>>> the output port knows which source it corresponds to, and it also
>>>> knows
>>>> which input port it corresponds to.
>>>
>>> Why do we need labels ? We have connection information for all devices
>>> (both in and out), so, why do we need this label to find a device ?
>>
>> Because our funnel's design has multi-output ports, the data stream
>> will not
>>
>> know which output port should pass in building the data trace path.
>> This source
>>
>> label can make the data stream find the right output port to go.
>>
>>>
>>> And also, I thought TPDM is a source device, why does a funnel output
>>> port link to a source ?
>>
>> No, this label doesn't mean this funnel output port link to a source,
>> it just let
>>
>> the output port know its data source.
>>
>>>
>>> Are these funnels programmable ? Or, are they static ? If they are
>>> static, do these need to be described in the DT ? If they are simply
>>> acting as a "LINK" (or HWFIFO ?)
>>
>> These funnels are static, and we will add the "label" to the DT to
>> describe the
>>
>> multi-output ports for these funnels.
>
> I think there is still a bit of confusion. By "Dynamic" I mean,
> the "dynamic funnel" (explicit port enablement via MMIO) vs "static
> funnel" (no programming, always ON).
>
> So, coming to your example, do we need to "explicitly" enable trace
> flow for an "input" and/or an "output" port in your "funnel" ?
Sorry for my misunderstanding in the previous mails. Our funnels are
programmable just like the common dynamic funnels.
In our solution, we just make funnels have multiple output ports
connected to different devices or ports. When we use it, we still
enable the input port through programming. Our solution is to know which
input port the expected data comes from based on the
source label corresponding to the output port. This way we can build the
expected trace path. In other respects, it is used the same
as common dynamic funnels.
Best,
Tao
>
>
>>
>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
>> what's the meaning
>
> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is acting
> like a cache for easing ATB data load, by providing h/w buffering.
> (In your case, it may not be providing any buffering, it doesn't matter
> either way, as it is not visible to the driver).
>
> Suzuki
>
>>
>> of this. Could you describe it in detail?
>>
>>
>> Best,
>>
>> Tao
>>
>>>
>>> Suzuki
>>>
>>>>
>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>> ---
>>>> drivers/hwtracing/coresight/coresight-core.c | 81
>>>> ++++++++++++++++---
>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>>>> include/linux/coresight.h | 2 +
>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>>> b/drivers/hwtracing/coresight/coresight-core.c
>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>> @@ -113,15 +113,63 @@ struct coresight_device
>>>> *coresight_get_percpu_sink(int cpu)
>>>> }
>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>>> +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;
>>>> +}
>>>> +
>>>> +/**
>>>> + * 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_label == NULL)
>>>> + return ret;
>>>> +
>>>> + source = coresight_get_source(path);
>>>> + if (source == NULL)
>>>> + return ret;
>>>> +
>>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>> + conn->source_label))
>>>> + ret = 0;
>>>> + else
>>>> + ret = -1;
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> static struct coresight_connection *
>>>> coresight_find_out_connection(struct coresight_device *src_dev,
>>>> - struct coresight_device *dest_dev)
>>>> + struct coresight_device *dest_dev,
>>>> + struct list_head *path)
>>>> {
>>>> int i;
>>>> struct coresight_connection *conn;
>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>> conn = src_dev->pdata->out_conns[i];
>>>> + if (coresight_source_filter(path, conn))
>>>> + continue;
>>>> if (conn->dest_dev == dest_dev)
>>>> return conn;
>>>> }
>>>> @@ -312,7 +360,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;
>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>>>> coresight_device *csdev,
>>>> if (!parent || !child)
>>>> return -EINVAL;
>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>>> link_subtype = csdev->subtype.link_subtype;
>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>>> IS_ERR(inconn))
>>>> @@ -341,7 +390,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;
>>>> int link_subtype;
>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>>>> coresight_device *csdev,
>>>> if (!parent || !child)
>>>> return;
>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>>> link_subtype = csdev->subtype.link_subtype;
>>>> if (link_ops(csdev)->disable) {
>>>> @@ -507,7 +557,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;
>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
>>>> *path, enum cs_mode mode,
>>>> 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;
>>>> @@ -802,7 +852,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;
>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>>>> coresight_device *csdev,
>>>> if (coresight_is_percpu_source(csdev) &&
>>>> coresight_is_percpu_sink(sink) &&
>>>> sink == per_cpu(csdev_sink,
>>>> source_ops(csdev)->cpu_id(csdev))) {
>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
>>>> + if (_coresight_build_path(sink, sink, path, source) == 0) {
>>>> found = true;
>>>> goto out;
>>>> }
>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>>>> coresight_device *csdev,
>>>> struct coresight_device *child_dev;
>>>> child_dev = csdev->pdata->out_conns[i]->dest_dev;
>>>> + if (csdev->pdata->out_conns[i]->source_label &&
>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>> + csdev->pdata->out_conns[i]->source_label))
>>>> + continue;
>>>> if (child_dev &&
>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>>> + _coresight_build_path(child_dev, sink, path, source)
>>>> == 0) {
>>>> found = true;
>>>> break;
>>>> }
>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
>>>> device *dev,
>>>> struct fwnode_handle *rdev_fwnode;
>>>> struct coresight_connection conn = {};
>>>> struct coresight_connection *new_conn;
>>>> + const char *label;
>>>> do {
>>>> /* Parse the local port details */
>>>> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
>>>> device *dev,
>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>>> conn.dest_port = rendpoint.port;
>>>> + conn.source_label = NULL;
>>>> + if (!of_property_read_string(ep, "label", &label))
>>>> + conn.source_label = label;
>>>> +
>>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
>>>> if (IS_ERR_VALUE(new_conn)) {
>>>> fwnode_handle_put(conn.dest_fwnode);
>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>>> index e8b6e388218c..a9c06ef9bbb2 100644
>>>> --- a/include/linux/coresight.h
>>>> +++ b/include/linux/coresight.h
>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
>>>> * struct coresight_connection - representation of a single
>>>> connection
>>>> * @src_port: a connection's output port number.
>>>> * @dest_port: destination's input port number @src_port is
>>>> connected to.
>>>> + * @source_label: source component's label.
>>>> * @dest_fwnode: destination component's fwnode handle.
>>>> * @dest_dev: a @coresight_device representation of the component
>>>> connected to @src_port. NULL until the device is created
>>>> @@ -195,6 +196,7 @@ struct coresight_desc {
>>>> struct coresight_connection {
>>>> int src_port;
>>>> int dest_port;
>>>> + const char *source_label;
>>>> struct fwnode_handle *dest_fwnode;
>>>> struct coresight_device *dest_dev;
>>>> struct coresight_sysfs_link *link;
>>>
>
On 09/04/2024 14:22, Tao Zhang wrote:
>
> On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
>> Hi
>>
>> On 29/03/2024 09:27, Tao Zhang wrote:
>>>
>>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
>>>> On 21/03/2024 08:32, Tao Zhang wrote:
>>>>> 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
>>>>> port is selected according to the source in the trace path.
>>>>>
>>>>> The source of the input port on funnels will be marked in the
>>>>> device tree.
>>>>> e.g.
>>>>> tpdm@xxxxxxx {
>>>>> ... ... ... ...
>>>>> };
>>>>>
>>>>> funnel_XXX: funnel@xxxxxxx {
>>>>> ... ... ... ...
>>>>> out-ports {
>>>>> ... ... ... ...
>>>>> port@x {
>>>>> ... ... ... ...
>>>>> label = "xxxxxxx.tpdm"; <-- To label the source
>>>>> }; corresponding to the output
>>>>> ... ... ... ... connection "port@x". And this
>>>>> }; is a hardware static
>>>>> connections.
>>>>> ... ... ... ... Here needs to refer to hardware
>>>>> }; design.
>>>>>
>>>>> Then driver will parse the source label marked in the device tree, and
>>>>> save it to the coresight path. When the function needs to know the
>>>>> source label, it could obtain it from coresight path parameter.
>>>>> Finally,
>>>>> the output port knows which source it corresponds to, and it also
>>>>> knows
>>>>> which input port it corresponds to.
>>>>
>>>> Why do we need labels ? We have connection information for all devices
>>>> (both in and out), so, why do we need this label to find a device ?
>>>
>>> Because our funnel's design has multi-output ports, the data stream
>>> will not
>>>
>>> know which output port should pass in building the data trace path.
>>> This source
>>>
>>> label can make the data stream find the right output port to go.
>>>
>>>>
>>>> And also, I thought TPDM is a source device, why does a funnel output
>>>> port link to a source ?
>>>
>>> No, this label doesn't mean this funnel output port link to a source,
>>> it just let
>>>
>>> the output port know its data source.
>>>
>>>>
>>>> Are these funnels programmable ? Or, are they static ? If they are
>>>> static, do these need to be described in the DT ? If they are simply
>>>> acting as a "LINK" (or HWFIFO ?)
>>>
>>> These funnels are static, and we will add the "label" to the DT to
>>> describe the
>>>
>>> multi-output ports for these funnels.
>>
>> I think there is still a bit of confusion. By "Dynamic" I mean,
>> the "dynamic funnel" (explicit port enablement via MMIO) vs "static
>> funnel" (no programming, always ON).
>>
>> So, coming to your example, do we need to "explicitly" enable trace
>> flow for an "input" and/or an "output" port in your "funnel" ?
>
> Sorry for my misunderstanding in the previous mails. Our funnels are
> programmable just like the common dynamic funnels.
>
> In our solution, we just make funnels have multiple output ports
> connected to different devices or ports. When we use it, we still
>
> enable the input port through programming. Our solution is to know which
> input port the expected data comes from based on the
>
> source label corresponding to the output port. This way we can build the
> expected trace path. In other respects, it is used the same
>
> as common dynamic funnels.
Ok. So, to summarise :
1. This is not a standard Funnel, but a trace link with multiple-input
and multiple-output, with inputs hardwired to an outline at
integration.
2. The programming model is same as that of a "standard funnel".
Now, we do have enough information in the coresight_connections
to traverse input/output ports. But we need additional logic
to "hardwire" the ports to each other and necessary logic
to handle the
There are two options here :
1. Treat this as a new component and have its own driver, with
additional logic to handle the input/output wiring.
2. Drive it using the funnel driver, with a a new compatible and
add additional logic to handle the input/output wiring.
My inclination is towards (2), we need to see how this works out.
We need to irrespective of the options, we need special handling
for hardwired ports in 1) building path 2) walking back the path
(in TPDA driver)
We also need some "DT" information to bind a given input port
to an output port. We must not use "any device" labels to hack
this up, like the approach in this series.
Rob/Krzysztof,
Do you have any recommendations for describing the 'hard wired
ports' ?
e.g:
component {
input_ports {
component_input_port0: port@0 {
...
<hard-wired-to*> = &component_output_port0;
};
...
};
output_ports {
componentne_output_port0: port@0 {
...
<hard-wired-to> = &component_input_port0;
};
...
};
};
*Need a better suitable property than "hard-wired-to".
Suzuki
>
>
> Best,
>
> Tao
>
>>
>>
>>>
>>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
>>> what's the meaning
>>
>> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is acting
>> like a cache for easing ATB data load, by providing h/w buffering.
>> (In your case, it may not be providing any buffering, it doesn't matter
>> either way, as it is not visible to the driver).
>>
>> Suzuki
>>
>>>
>>> of this. Could you describe it in detail?
>>>
>>>
>>> Best,
>>>
>>> Tao
>>>
>>>>
>>>> Suzuki
>>>>
>>>>>
>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>> ---
>>>>> drivers/hwtracing/coresight/coresight-core.c | 81
>>>>> ++++++++++++++++---
>>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>>>>> include/linux/coresight.h | 2 +
>>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>>>> b/drivers/hwtracing/coresight/coresight-core.c
>>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>>> @@ -113,15 +113,63 @@ struct coresight_device
>>>>> *coresight_get_percpu_sink(int cpu)
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>>>> +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;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * 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_label == NULL)
>>>>> + return ret;
>>>>> +
>>>>> + source = coresight_get_source(path);
>>>>> + if (source == NULL)
>>>>> + return ret;
>>>>> +
>>>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>> + conn->source_label))
>>>>> + ret = 0;
>>>>> + else
>>>>> + ret = -1;
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> static struct coresight_connection *
>>>>> coresight_find_out_connection(struct coresight_device *src_dev,
>>>>> - struct coresight_device *dest_dev)
>>>>> + struct coresight_device *dest_dev,
>>>>> + struct list_head *path)
>>>>> {
>>>>> int i;
>>>>> struct coresight_connection *conn;
>>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>>> conn = src_dev->pdata->out_conns[i];
>>>>> + if (coresight_source_filter(path, conn))
>>>>> + continue;
>>>>> if (conn->dest_dev == dest_dev)
>>>>> return conn;
>>>>> }
>>>>> @@ -312,7 +360,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;
>>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>>>>> coresight_device *csdev,
>>>>> if (!parent || !child)
>>>>> return -EINVAL;
>>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>>>> IS_ERR(inconn))
>>>>> @@ -341,7 +390,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;
>>>>> int link_subtype;
>>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>>>>> coresight_device *csdev,
>>>>> if (!parent || !child)
>>>>> return;
>>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>> if (link_ops(csdev)->disable) {
>>>>> @@ -507,7 +557,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;
>>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
>>>>> *path, enum cs_mode mode,
>>>>> 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;
>>>>> @@ -802,7 +852,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;
>>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>>>>> coresight_device *csdev,
>>>>> if (coresight_is_percpu_source(csdev) &&
>>>>> coresight_is_percpu_sink(sink) &&
>>>>> sink == per_cpu(csdev_sink,
>>>>> source_ops(csdev)->cpu_id(csdev))) {
>>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
>>>>> + if (_coresight_build_path(sink, sink, path, source) == 0) {
>>>>> found = true;
>>>>> goto out;
>>>>> }
>>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>>>>> coresight_device *csdev,
>>>>> struct coresight_device *child_dev;
>>>>> child_dev = csdev->pdata->out_conns[i]->dest_dev;
>>>>> + if (csdev->pdata->out_conns[i]->source_label &&
>>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>> + csdev->pdata->out_conns[i]->source_label))
>>>>> + continue;
>>>>> if (child_dev &&
>>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>>>> + _coresight_build_path(child_dev, sink, path, source)
>>>>> == 0) {
>>>>> found = true;
>>>>> break;
>>>>> }
>>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>>> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
>>>>> device *dev,
>>>>> struct fwnode_handle *rdev_fwnode;
>>>>> struct coresight_connection conn = {};
>>>>> struct coresight_connection *new_conn;
>>>>> + const char *label;
>>>>> do {
>>>>> /* Parse the local port details */
>>>>> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
>>>>> device *dev,
>>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>>>> conn.dest_port = rendpoint.port;
>>>>> + conn.source_label = NULL;
>>>>> + if (!of_property_read_string(ep, "label", &label))
>>>>> + conn.source_label = label;
>>>>> +
>>>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
>>>>> if (IS_ERR_VALUE(new_conn)) {
>>>>> fwnode_handle_put(conn.dest_fwnode);
>>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>>>> index e8b6e388218c..a9c06ef9bbb2 100644
>>>>> --- a/include/linux/coresight.h
>>>>> +++ b/include/linux/coresight.h
>>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
>>>>> * struct coresight_connection - representation of a single
>>>>> connection
>>>>> * @src_port: a connection's output port number.
>>>>> * @dest_port: destination's input port number @src_port is
>>>>> connected to.
>>>>> + * @source_label: source component's label.
>>>>> * @dest_fwnode: destination component's fwnode handle.
>>>>> * @dest_dev: a @coresight_device representation of the component
>>>>> connected to @src_port. NULL until the device is created
>>>>> @@ -195,6 +196,7 @@ struct coresight_desc {
>>>>> struct coresight_connection {
>>>>> int src_port;
>>>>> int dest_port;
>>>>> + const char *source_label;
>>>>> struct fwnode_handle *dest_fwnode;
>>>>> struct coresight_device *dest_dev;
>>>>> struct coresight_sysfs_link *link;
>>>>
>>
Hi,
On Mon, 15 Apr 2024 at 14:24, Suzuki K Poulose <[email protected]> wrote:
>
> On 09/04/2024 14:22, Tao Zhang wrote:
> >
> > On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
> >> Hi
> >>
> >> On 29/03/2024 09:27, Tao Zhang wrote:
> >>>
> >>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
> >>>> On 21/03/2024 08:32, Tao Zhang wrote:
> >>>>> 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
> >>>>> port is selected according to the source in the trace path.
> >>>>>
> >>>>> The source of the input port on funnels will be marked in the
> >>>>> device tree.
> >>>>> e.g.
> >>>>> tpdm@xxxxxxx {
> >>>>> ... ... ... ...
> >>>>> };
> >>>>>
> >>>>> funnel_XXX: funnel@xxxxxxx {
> >>>>> ... ... ... ...
> >>>>> out-ports {
> >>>>> ... ... ... ...
> >>>>> port@x {
> >>>>> ... ... ... ...
> >>>>> label = "xxxxxxx.tpdm"; <-- To label the source
> >>>>> }; corresponding to the output
> >>>>> ... ... ... ... connection "port@x". And this
> >>>>> }; is a hardware static
> >>>>> connections.
> >>>>> ... ... ... ... Here needs to refer to hardware
> >>>>> }; design.
> >>>>>
> >>>>> Then driver will parse the source label marked in the device tree, and
> >>>>> save it to the coresight path. When the function needs to know the
> >>>>> source label, it could obtain it from coresight path parameter.
> >>>>> Finally,
> >>>>> the output port knows which source it corresponds to, and it also
> >>>>> knows
> >>>>> which input port it corresponds to.
> >>>>
> >>>> Why do we need labels ? We have connection information for all devices
> >>>> (both in and out), so, why do we need this label to find a device ?
> >>>
> >>> Because our funnel's design has multi-output ports, the data stream
> >>> will not
> >>>
> >>> know which output port should pass in building the data trace path.
> >>> This source
> >>>
> >>> label can make the data stream find the right output port to go.
> >>>
> >>>>
> >>>> And also, I thought TPDM is a source device, why does a funnel output
> >>>> port link to a source ?
> >>>
> >>> No, this label doesn't mean this funnel output port link to a source,
> >>> it just let
> >>>
> >>> the output port know its data source.
> >>>
> >>>>
> >>>> Are these funnels programmable ? Or, are they static ? If they are
> >>>> static, do these need to be described in the DT ? If they are simply
> >>>> acting as a "LINK" (or HWFIFO ?)
> >>>
> >>> These funnels are static, and we will add the "label" to the DT to
> >>> describe the
> >>>
> >>> multi-output ports for these funnels.
> >>
> >> I think there is still a bit of confusion. By "Dynamic" I mean,
> >> the "dynamic funnel" (explicit port enablement via MMIO) vs "static
> >> funnel" (no programming, always ON).
> >>
> >> So, coming to your example, do we need to "explicitly" enable trace
> >> flow for an "input" and/or an "output" port in your "funnel" ?
> >
> > Sorry for my misunderstanding in the previous mails. Our funnels are
> > programmable just like the common dynamic funnels.
> >
> > In our solution, we just make funnels have multiple output ports
> > connected to different devices or ports. When we use it, we still
> >
> > enable the input port through programming. Our solution is to know which
> > input port the expected data comes from based on the
> >
> > source label corresponding to the output port. This way we can build the
> > expected trace path. In other respects, it is used the same
> >
> > as common dynamic funnels.
>
>
> Ok. So, to summarise :
>
> 1. This is not a standard Funnel, but a trace link with multiple-input
> and multiple-output, with inputs hardwired to an outline at
> integration.
> 2. The programming model is same as that of a "standard funnel".
>
> Now, we do have enough information in the coresight_connections
> to traverse input/output ports. But we need additional logic
> to "hardwire" the ports to each other and necessary logic
> to handle the
>
> There are two options here :
>
> 1. Treat this as a new component and have its own driver, with
> additional logic to handle the input/output wiring.
>
> 2. Drive it using the funnel driver, with a a new compatible and
> add additional logic to handle the input/output wiring.
>
> My inclination is towards (2), we need to see how this works out.
>
> We need to irrespective of the options, we need special handling
> for hardwired ports in 1) building path 2) walking back the path
> (in TPDA driver)
>
> We also need some "DT" information to bind a given input port
> to an output port. We must not use "any device" labels to hack
> this up, like the approach in this series.
>
Given that the internal connections are static for the given device,
then the compatible will imply these connections in just the same way
as the arm,coresight-funnel implies that all inputs are connected to
the single output.
Irrespective of if a new driver is used, or an extension to the
current funnel driver to handle a new compatible - the mapping between
input and output ports are created based on the compatible..
As we are building a path from source to sink, what is then needed is
a method in the generic path building code, to recognise these
amppings and filter the output ports that are searched based on the
input port in use.
On standard components, where the mapping is not present, then the
code will continue as it does now, for these compound funnels, the
mappings will be present and the output filtering will occur.
This removes the need for the labels / extra connection attributes on
devices other than the funnel, and also removes the need to specify
the internal connections as part of the device tree.
Regards
Mike
> Rob/Krzysztof,
>
> Do you have any recommendations for describing the 'hard wired
> ports' ?
>
> e.g:
>
> component {
> input_ports {
> component_input_port0: port@0 {
> ...
> <hard-wired-to*> = &component_output_port0;
> };
> ...
> };
>
> output_ports {
> componentne_output_port0: port@0 {
> ...
> <hard-wired-to> = &component_input_port0;
> };
> ...
> };
>
> };
>
> *Need a better suitable property than "hard-wired-to".
>
>
> Suzuki
>
>
> >
> >
> > Best,
> >
> > Tao
> >
> >>
> >>
> >>>
> >>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
> >>> what's the meaning
> >>
> >> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is acting
> >> like a cache for easing ATB data load, by providing h/w buffering.
> >> (In your case, it may not be providing any buffering, it doesn't matter
> >> either way, as it is not visible to the driver).
> >>
> >> Suzuki
> >>
> >>>
> >>> of this. Could you describe it in detail?
> >>>
> >>>
> >>> Best,
> >>>
> >>> Tao
> >>>
> >>>>
> >>>> Suzuki
> >>>>
> >>>>>
> >>>>> Signed-off-by: Tao Zhang <[email protected]>
> >>>>> ---
> >>>>> drivers/hwtracing/coresight/coresight-core.c | 81
> >>>>> ++++++++++++++++---
> >>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
> >>>>> include/linux/coresight.h | 2 +
> >>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
> >>>>> b/drivers/hwtracing/coresight/coresight-core.c
> >>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
> >>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
> >>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
> >>>>> @@ -113,15 +113,63 @@ struct coresight_device
> >>>>> *coresight_get_percpu_sink(int cpu)
> >>>>> }
> >>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
> >>>>> +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;
> >>>>> +}
> >>>>> +
> >>>>> +/**
> >>>>> + * 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_label == NULL)
> >>>>> + return ret;
> >>>>> +
> >>>>> + source = coresight_get_source(path);
> >>>>> + if (source == NULL)
> >>>>> + return ret;
> >>>>> +
> >>>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >>>>> + conn->source_label))
> >>>>> + ret = 0;
> >>>>> + else
> >>>>> + ret = -1;
> >>>>> +
> >>>>> + return ret;
> >>>>> +}
> >>>>> +
> >>>>> static struct coresight_connection *
> >>>>> coresight_find_out_connection(struct coresight_device *src_dev,
> >>>>> - struct coresight_device *dest_dev)
> >>>>> + struct coresight_device *dest_dev,
> >>>>> + struct list_head *path)
> >>>>> {
> >>>>> int i;
> >>>>> struct coresight_connection *conn;
> >>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
> >>>>> conn = src_dev->pdata->out_conns[i];
> >>>>> + if (coresight_source_filter(path, conn))
> >>>>> + continue;
> >>>>> if (conn->dest_dev == dest_dev)
> >>>>> return conn;
> >>>>> }
> >>>>> @@ -312,7 +360,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;
> >>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
> >>>>> coresight_device *csdev,
> >>>>> if (!parent || !child)
> >>>>> return -EINVAL;
> >>>>> - inconn = coresight_find_out_connection(parent, csdev);
> >>>>> - outconn = coresight_find_out_connection(csdev, child);
> >>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
> >>>>> + outconn = coresight_find_out_connection(csdev, child, path);
> >>>>> link_subtype = csdev->subtype.link_subtype;
> >>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
> >>>>> IS_ERR(inconn))
> >>>>> @@ -341,7 +390,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;
> >>>>> int link_subtype;
> >>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
> >>>>> coresight_device *csdev,
> >>>>> if (!parent || !child)
> >>>>> return;
> >>>>> - inconn = coresight_find_out_connection(parent, csdev);
> >>>>> - outconn = coresight_find_out_connection(csdev, child);
> >>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
> >>>>> + outconn = coresight_find_out_connection(csdev, child, path);
> >>>>> link_subtype = csdev->subtype.link_subtype;
> >>>>> if (link_ops(csdev)->disable) {
> >>>>> @@ -507,7 +557,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;
> >>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
> >>>>> *path, enum cs_mode mode,
> >>>>> 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;
> >>>>> @@ -802,7 +852,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;
> >>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
> >>>>> coresight_device *csdev,
> >>>>> if (coresight_is_percpu_source(csdev) &&
> >>>>> coresight_is_percpu_sink(sink) &&
> >>>>> sink == per_cpu(csdev_sink,
> >>>>> source_ops(csdev)->cpu_id(csdev))) {
> >>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
> >>>>> + if (_coresight_build_path(sink, sink, path, source) == 0) {
> >>>>> found = true;
> >>>>> goto out;
> >>>>> }
> >>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
> >>>>> coresight_device *csdev,
> >>>>> struct coresight_device *child_dev;
> >>>>> child_dev = csdev->pdata->out_conns[i]->dest_dev;
> >>>>> + if (csdev->pdata->out_conns[i]->source_label &&
> >>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >>>>> + csdev->pdata->out_conns[i]->source_label))
> >>>>> + continue;
> >>>>> if (child_dev &&
> >>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
> >>>>> + _coresight_build_path(child_dev, sink, path, source)
> >>>>> == 0) {
> >>>>> found = true;
> >>>>> break;
> >>>>> }
> >>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
> >>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
> >>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> >>>>> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
> >>>>> device *dev,
> >>>>> struct fwnode_handle *rdev_fwnode;
> >>>>> struct coresight_connection conn = {};
> >>>>> struct coresight_connection *new_conn;
> >>>>> + const char *label;
> >>>>> do {
> >>>>> /* Parse the local port details */
> >>>>> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
> >>>>> device *dev,
> >>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
> >>>>> conn.dest_port = rendpoint.port;
> >>>>> + conn.source_label = NULL;
> >>>>> + if (!of_property_read_string(ep, "label", &label))
> >>>>> + conn.source_label = label;
> >>>>> +
> >>>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
> >>>>> if (IS_ERR_VALUE(new_conn)) {
> >>>>> fwnode_handle_put(conn.dest_fwnode);
> >>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> >>>>> index e8b6e388218c..a9c06ef9bbb2 100644
> >>>>> --- a/include/linux/coresight.h
> >>>>> +++ b/include/linux/coresight.h
> >>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
> >>>>> * struct coresight_connection - representation of a single
> >>>>> connection
> >>>>> * @src_port: a connection's output port number.
> >>>>> * @dest_port: destination's input port number @src_port is
> >>>>> connected to.
> >>>>> + * @source_label: source component's label.
> >>>>> * @dest_fwnode: destination component's fwnode handle.
> >>>>> * @dest_dev: a @coresight_device representation of the component
> >>>>> connected to @src_port. NULL until the device is created
> >>>>> @@ -195,6 +196,7 @@ struct coresight_desc {
> >>>>> struct coresight_connection {
> >>>>> int src_port;
> >>>>> int dest_port;
> >>>>> + const char *source_label;
> >>>>> struct fwnode_handle *dest_fwnode;
> >>>>> struct coresight_device *dest_dev;
> >>>>> struct coresight_sysfs_link *link;
> >>>>
> >>
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Hi Mike
On 16/04/2024 15:19, Mike Leach wrote:
> Hi,
>
> On Mon, 15 Apr 2024 at 14:24, Suzuki K Poulose <[email protected]> wrote:
>>
>> On 09/04/2024 14:22, Tao Zhang wrote:
>>>
>>> On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
>>>> Hi
>>>>
>>>> On 29/03/2024 09:27, Tao Zhang wrote:
>>>>>
>>>>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
>>>>>> On 21/03/2024 08:32, Tao Zhang wrote:
>>>>>>> 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
>>>>>>> port is selected according to the source in the trace path.
>>>>>>>
>>>>>>> The source of the input port on funnels will be marked in the
>>>>>>> device tree.
>>>>>>> e.g.
>>>>>>> tpdm@xxxxxxx {
>>>>>>> ... ... ... ...
>>>>>>> };
>>>>>>>
>>>>>>> funnel_XXX: funnel@xxxxxxx {
>>>>>>> ... ... ... ...
>>>>>>> out-ports {
>>>>>>> ... ... ... ...
>>>>>>> port@x {
>>>>>>> ... ... ... ...
>>>>>>> label = "xxxxxxx.tpdm"; <-- To label the source
>>>>>>> }; corresponding to the output
>>>>>>> ... ... ... ... connection "port@x". And this
>>>>>>> }; is a hardware static
>>>>>>> connections.
>>>>>>> ... ... ... ... Here needs to refer to hardware
>>>>>>> }; design.
>>>>>>>
>>>>>>> Then driver will parse the source label marked in the device tree, and
>>>>>>> save it to the coresight path. When the function needs to know the
>>>>>>> source label, it could obtain it from coresight path parameter.
>>>>>>> Finally,
>>>>>>> the output port knows which source it corresponds to, and it also
>>>>>>> knows
>>>>>>> which input port it corresponds to.
>>>>>>
>>>>>> Why do we need labels ? We have connection information for all devices
>>>>>> (both in and out), so, why do we need this label to find a device ?
>>>>>
>>>>> Because our funnel's design has multi-output ports, the data stream
>>>>> will not
>>>>>
>>>>> know which output port should pass in building the data trace path.
>>>>> This source
>>>>>
>>>>> label can make the data stream find the right output port to go.
>>>>>
>>>>>>
>>>>>> And also, I thought TPDM is a source device, why does a funnel output
>>>>>> port link to a source ?
>>>>>
>>>>> No, this label doesn't mean this funnel output port link to a source,
>>>>> it just let
>>>>>
>>>>> the output port know its data source.
>>>>>
>>>>>>
>>>>>> Are these funnels programmable ? Or, are they static ? If they are
>>>>>> static, do these need to be described in the DT ? If they are simply
>>>>>> acting as a "LINK" (or HWFIFO ?)
>>>>>
>>>>> These funnels are static, and we will add the "label" to the DT to
>>>>> describe the
>>>>>
>>>>> multi-output ports for these funnels.
>>>>
>>>> I think there is still a bit of confusion. By "Dynamic" I mean,
>>>> the "dynamic funnel" (explicit port enablement via MMIO) vs "static
>>>> funnel" (no programming, always ON).
>>>>
>>>> So, coming to your example, do we need to "explicitly" enable trace
>>>> flow for an "input" and/or an "output" port in your "funnel" ?
>>>
>>> Sorry for my misunderstanding in the previous mails. Our funnels are
>>> programmable just like the common dynamic funnels.
>>>
>>> In our solution, we just make funnels have multiple output ports
>>> connected to different devices or ports. When we use it, we still
>>>
>>> enable the input port through programming. Our solution is to know which
>>> input port the expected data comes from based on the
>>>
>>> source label corresponding to the output port. This way we can build the
>>> expected trace path. In other respects, it is used the same
>>>
>>> as common dynamic funnels.
>>
>>
>> Ok. So, to summarise :
>>
>> 1. This is not a standard Funnel, but a trace link with multiple-input
>> and multiple-output, with inputs hardwired to an outline at
>> integration.
>> 2. The programming model is same as that of a "standard funnel".
>>
>> Now, we do have enough information in the coresight_connections
>> to traverse input/output ports. But we need additional logic
>> to "hardwire" the ports to each other and necessary logic
>> to handle the
>>
>> There are two options here :
>>
>> 1. Treat this as a new component and have its own driver, with
>> additional logic to handle the input/output wiring.
>>
>> 2. Drive it using the funnel driver, with a a new compatible and
>> add additional logic to handle the input/output wiring.
>>
>> My inclination is towards (2), we need to see how this works out.
>>
>> We need to irrespective of the options, we need special handling
>> for hardwired ports in 1) building path 2) walking back the path
>> (in TPDA driver)
>>
>> We also need some "DT" information to bind a given input port
>> to an output port. We must not use "any device" labels to hack
>> this up, like the approach in this series.
>>
>
> Given that the internal connections are static for the given device,
> then the compatible will imply these connections in just the same way
> as the arm,coresight-funnel implies that all inputs are connected to
> the single output.
I am sorry, I couldn't follow the last part. We have two or more output
ports and we need a way to identify, which input port is hardwired to
output-port0 and output-port1. Given we need special handling for these
anyway, I would like to avoid hard coding the input-output connection.
i.e., we do not want to assume that input-0 is always => output-0.
>
> Irrespective of if a new driver is used, or an extension to the
> current funnel driver to handle a new compatible - the mapping between
> input and output ports are created based on the compatible..
>
> As we are building a path from source to sink, what is then needed is
> a method in the generic path building code, to recognise these
> amppings and filter the output ports that are searched based on the
> input port in use.
Agreed. We could mark this as a property of the
port/coresight_connection.
>
> On standard components, where the mapping is not present, then the
> code will continue as it does now, for these compound funnels, the
> mappings will be present and the output filtering will occur.
Agreed
> This removes the need for the labels / extra connection attributes on
> devices other than the funnel, and also removes the need to specify
> the internal connections as part of the device tree.
I am still not clear how we map the input-output ports. Rest is what
exactly I had in mind. So, once we sort out the port mapping
we could proceed to the prototyping.
Kind regards
Suzuki
>
> Regards
>
> Mike
>
>> Rob/Krzysztof,
>>
>> Do you have any recommendations for describing the 'hard wired
>> ports' ?
>>
>> e.g:
>>
>> component {
>> input_ports {
>> component_input_port0: port@0 {
>> ...
>> <hard-wired-to*> = &component_output_port0;
>> };
>> ...
>> };
>>
>> output_ports {
>> componentne_output_port0: port@0 {
>> ...
>> <hard-wired-to> = &component_input_port0;
>> };
>> ...
>> };
>>
>> };
>>
>> *Need a better suitable property than "hard-wired-to".
>>
>>
>> Suzuki
>>
>>
>>>
>>>
>>> Best,
>>>
>>> Tao
>>>
>>>>
>>>>
>>>>>
>>>>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
>>>>> what's the meaning
>>>>
>>>> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is acting
>>>> like a cache for easing ATB data load, by providing h/w buffering.
>>>> (In your case, it may not be providing any buffering, it doesn't matter
>>>> either way, as it is not visible to the driver).
>>>>
>>>> Suzuki
>>>>
>>>>>
>>>>> of this. Could you describe it in detail?
>>>>>
>>>>>
>>>>> Best,
>>>>>
>>>>> Tao
>>>>>
>>>>>>
>>>>>> Suzuki
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>> ---
>>>>>>> drivers/hwtracing/coresight/coresight-core.c | 81
>>>>>>> ++++++++++++++++---
>>>>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>>>>>>> include/linux/coresight.h | 2 +
>>>>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>> b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
>>>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>> @@ -113,15 +113,63 @@ struct coresight_device
>>>>>>> *coresight_get_percpu_sink(int cpu)
>>>>>>> }
>>>>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>>>>>> +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;
>>>>>>> +}
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * 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_label == NULL)
>>>>>>> + return ret;
>>>>>>> +
>>>>>>> + source = coresight_get_source(path);
>>>>>>> + if (source == NULL)
>>>>>>> + return ret;
>>>>>>> +
>>>>>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>>>> + conn->source_label))
>>>>>>> + ret = 0;
>>>>>>> + else
>>>>>>> + ret = -1;
>>>>>>> +
>>>>>>> + return ret;
>>>>>>> +}
>>>>>>> +
>>>>>>> static struct coresight_connection *
>>>>>>> coresight_find_out_connection(struct coresight_device *src_dev,
>>>>>>> - struct coresight_device *dest_dev)
>>>>>>> + struct coresight_device *dest_dev,
>>>>>>> + struct list_head *path)
>>>>>>> {
>>>>>>> int i;
>>>>>>> struct coresight_connection *conn;
>>>>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>>>>> conn = src_dev->pdata->out_conns[i];
>>>>>>> + if (coresight_source_filter(path, conn))
>>>>>>> + continue;
>>>>>>> if (conn->dest_dev == dest_dev)
>>>>>>> return conn;
>>>>>>> }
>>>>>>> @@ -312,7 +360,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;
>>>>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>>>>>>> coresight_device *csdev,
>>>>>>> if (!parent || !child)
>>>>>>> return -EINVAL;
>>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>>>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>>>>>> IS_ERR(inconn))
>>>>>>> @@ -341,7 +390,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;
>>>>>>> int link_subtype;
>>>>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>>>>>>> coresight_device *csdev,
>>>>>>> if (!parent || !child)
>>>>>>> return;
>>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>>>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>> if (link_ops(csdev)->disable) {
>>>>>>> @@ -507,7 +557,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;
>>>>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
>>>>>>> *path, enum cs_mode mode,
>>>>>>> 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;
>>>>>>> @@ -802,7 +852,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;
>>>>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>>>>>>> coresight_device *csdev,
>>>>>>> if (coresight_is_percpu_source(csdev) &&
>>>>>>> coresight_is_percpu_sink(sink) &&
>>>>>>> sink == per_cpu(csdev_sink,
>>>>>>> source_ops(csdev)->cpu_id(csdev))) {
>>>>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
>>>>>>> + if (_coresight_build_path(sink, sink, path, source) == 0) {
>>>>>>> found = true;
>>>>>>> goto out;
>>>>>>> }
>>>>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>>>>>>> coresight_device *csdev,
>>>>>>> struct coresight_device *child_dev;
>>>>>>> child_dev = csdev->pdata->out_conns[i]->dest_dev;
>>>>>>> + if (csdev->pdata->out_conns[i]->source_label &&
>>>>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>>>> + csdev->pdata->out_conns[i]->source_label))
>>>>>>> + continue;
>>>>>>> if (child_dev &&
>>>>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>>>>>> + _coresight_build_path(child_dev, sink, path, source)
>>>>>>> == 0) {
>>>>>>> found = true;
>>>>>>> break;
>>>>>>> }
>>>>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
>>>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
>>>>>>> device *dev,
>>>>>>> struct fwnode_handle *rdev_fwnode;
>>>>>>> struct coresight_connection conn = {};
>>>>>>> struct coresight_connection *new_conn;
>>>>>>> + const char *label;
>>>>>>> do {
>>>>>>> /* Parse the local port details */
>>>>>>> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
>>>>>>> device *dev,
>>>>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>>>>>> conn.dest_port = rendpoint.port;
>>>>>>> + conn.source_label = NULL;
>>>>>>> + if (!of_property_read_string(ep, "label", &label))
>>>>>>> + conn.source_label = label;
>>>>>>> +
>>>>>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
>>>>>>> if (IS_ERR_VALUE(new_conn)) {
>>>>>>> fwnode_handle_put(conn.dest_fwnode);
>>>>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>>>>>> index e8b6e388218c..a9c06ef9bbb2 100644
>>>>>>> --- a/include/linux/coresight.h
>>>>>>> +++ b/include/linux/coresight.h
>>>>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
>>>>>>> * struct coresight_connection - representation of a single
>>>>>>> connection
>>>>>>> * @src_port: a connection's output port number.
>>>>>>> * @dest_port: destination's input port number @src_port is
>>>>>>> connected to.
>>>>>>> + * @source_label: source component's label.
>>>>>>> * @dest_fwnode: destination component's fwnode handle.
>>>>>>> * @dest_dev: a @coresight_device representation of the component
>>>>>>> connected to @src_port. NULL until the device is created
>>>>>>> @@ -195,6 +196,7 @@ struct coresight_desc {
>>>>>>> struct coresight_connection {
>>>>>>> int src_port;
>>>>>>> int dest_port;
>>>>>>> + const char *source_label;
>>>>>>> struct fwnode_handle *dest_fwnode;
>>>>>>> struct coresight_device *dest_dev;
>>>>>>> struct coresight_sysfs_link *link;
>>>>>>
>>>>
>>
>
>
Hi Suzuki
On Wed, 17 Apr 2024 at 10:21, Suzuki K Poulose <[email protected]> wrote:
>
> Hi Mike
>
> On 16/04/2024 15:19, Mike Leach wrote:
> > Hi,
> >
> > On Mon, 15 Apr 2024 at 14:24, Suzuki K Poulose <[email protected]> wrote:
> >>
> >> On 09/04/2024 14:22, Tao Zhang wrote:
> >>>
> >>> On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
> >>>> Hi
> >>>>
> >>>> On 29/03/2024 09:27, Tao Zhang wrote:
> >>>>>
> >>>>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
> >>>>>> On 21/03/2024 08:32, Tao Zhang wrote:
> >>>>>>> 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
> >>>>>>> port is selected according to the source in the trace path.
> >>>>>>>
> >>>>>>> The source of the input port on funnels will be marked in the
> >>>>>>> device tree.
> >>>>>>> e.g.
> >>>>>>> tpdm@xxxxxxx {
> >>>>>>> ... ... ... ...
> >>>>>>> };
> >>>>>>>
> >>>>>>> funnel_XXX: funnel@xxxxxxx {
> >>>>>>> ... ... ... ...
> >>>>>>> out-ports {
> >>>>>>> ... ... ... ...
> >>>>>>> port@x {
> >>>>>>> ... ... ... ...
> >>>>>>> label = "xxxxxxx.tpdm"; <-- To label the source
> >>>>>>> }; corresponding to the output
> >>>>>>> ... ... ... ... connection "port@x". And this
> >>>>>>> }; is a hardware static
> >>>>>>> connections.
> >>>>>>> ... ... ... ... Here needs to refer to hardware
> >>>>>>> }; design.
> >>>>>>>
> >>>>>>> Then driver will parse the source label marked in the device tree, and
> >>>>>>> save it to the coresight path. When the function needs to know the
> >>>>>>> source label, it could obtain it from coresight path parameter.
> >>>>>>> Finally,
> >>>>>>> the output port knows which source it corresponds to, and it also
> >>>>>>> knows
> >>>>>>> which input port it corresponds to.
> >>>>>>
> >>>>>> Why do we need labels ? We have connection information for all devices
> >>>>>> (both in and out), so, why do we need this label to find a device ?
> >>>>>
> >>>>> Because our funnel's design has multi-output ports, the data stream
> >>>>> will not
> >>>>>
> >>>>> know which output port should pass in building the data trace path.
> >>>>> This source
> >>>>>
> >>>>> label can make the data stream find the right output port to go.
> >>>>>
> >>>>>>
> >>>>>> And also, I thought TPDM is a source device, why does a funnel output
> >>>>>> port link to a source ?
> >>>>>
> >>>>> No, this label doesn't mean this funnel output port link to a source,
> >>>>> it just let
> >>>>>
> >>>>> the output port know its data source.
> >>>>>
> >>>>>>
> >>>>>> Are these funnels programmable ? Or, are they static ? If they are
> >>>>>> static, do these need to be described in the DT ? If they are simply
> >>>>>> acting as a "LINK" (or HWFIFO ?)
> >>>>>
> >>>>> These funnels are static, and we will add the "label" to the DT to
> >>>>> describe the
> >>>>>
> >>>>> multi-output ports for these funnels.
> >>>>
> >>>> I think there is still a bit of confusion. By "Dynamic" I mean,
> >>>> the "dynamic funnel" (explicit port enablement via MMIO) vs "static
> >>>> funnel" (no programming, always ON).
> >>>>
> >>>> So, coming to your example, do we need to "explicitly" enable trace
> >>>> flow for an "input" and/or an "output" port in your "funnel" ?
> >>>
> >>> Sorry for my misunderstanding in the previous mails. Our funnels are
> >>> programmable just like the common dynamic funnels.
> >>>
> >>> In our solution, we just make funnels have multiple output ports
> >>> connected to different devices or ports. When we use it, we still
> >>>
> >>> enable the input port through programming. Our solution is to know which
> >>> input port the expected data comes from based on the
> >>>
> >>> source label corresponding to the output port. This way we can build the
> >>> expected trace path. In other respects, it is used the same
> >>>
> >>> as common dynamic funnels.
> >>
> >>
> >> Ok. So, to summarise :
> >>
> >> 1. This is not a standard Funnel, but a trace link with multiple-input
> >> and multiple-output, with inputs hardwired to an outline at
> >> integration.
> >> 2. The programming model is same as that of a "standard funnel".
> >>
> >> Now, we do have enough information in the coresight_connections
> >> to traverse input/output ports. But we need additional logic
> >> to "hardwire" the ports to each other and necessary logic
> >> to handle the
> >>
> >> There are two options here :
> >>
> >> 1. Treat this as a new component and have its own driver, with
> >> additional logic to handle the input/output wiring.
> >>
> >> 2. Drive it using the funnel driver, with a a new compatible and
> >> add additional logic to handle the input/output wiring.
> >>
> >> My inclination is towards (2), we need to see how this works out.
> >>
> >> We need to irrespective of the options, we need special handling
> >> for hardwired ports in 1) building path 2) walking back the path
> >> (in TPDA driver)
> >>
> >> We also need some "DT" information to bind a given input port
> >> to an output port. We must not use "any device" labels to hack
> >> this up, like the approach in this series.
> >>
> >
> > Given that the internal connections are static for the given device,
> > then the compatible will imply these connections in just the same way
> > as the arm,coresight-funnel implies that all inputs are connected to
> > the single output.
>
> I am sorry, I couldn't follow the last part. We have two or more output
> ports and we need a way to identify, which input port is hardwired to
> output-port0 and output-port1. Given we need special handling for these
> anyway, I would like to avoid hard coding the input-output connection.
> i.e., we do not want to assume that input-0 is always => output-0.
>
If we regard the current component as having compatible
"qcom,coresight-compound-funnel-v1", then this identifies the
relationship between the in-ports and out-ports.
So the new driver / extension to the funnel driver that handles this
compatible with know the static mapping between input and output so
program it.
If however you want a more generic approach to handle future different
versions of the component, then of course a method in DT of mapping
in-ports to out-ports is useful.
If did wonder if something along the lines of:-
compound-funnel@0x1234000 {
compatible = "compound-funnel"
regs = < 0x1234000 0x1000>
sub-funnel@0 {
in-ports {
[some port definitions]
}
out-ports {
[some port definitions]
}
}
sub-funnel@1 {
in-ports {
[some port definitions]
}
out-ports {
[some port definitions]
}
}
}
might be made to work? not sure about the implications of having more
that one set of in-ports and out-ports in a component in the device
tree & would need specific handling in the driver to iterate
sub-funnels.
>
> >
> > Irrespective of if a new driver is used, or an extension to the
> > current funnel driver to handle a new compatible - the mapping between
> > input and output ports are created based on the compatible..
> >
> > As we are building a path from source to sink, what is then needed is
> > a method in the generic path building code, to recognise these
> > amppings and filter the output ports that are searched based on the
> > input port in use.
>
> Agreed. We could mark this as a property of the
> port/coresight_connection.
>
> >
> > On standard components, where the mapping is not present, then the
> > code will continue as it does now, for these compound funnels, the
> > mappings will be present and the output filtering will occur.
>
> Agreed
>
> > This removes the need for the labels / extra connection attributes on
> > devices other than the funnel, and also removes the need to specify
> > the internal connections as part of the device tree.
>
> I am still not clear how we map the input-output ports. Rest is what
> exactly I had in mind. So, once we sort out the port mapping
> we could proceed to the prototyping.
>
given we iterate by output port index into an array of out ports, and
have an array of in-ports by index, a third mapping array, same size
as in-ports, determining the associated out port index should suffice.
Mapping array should be optional - if not there, path discovery works
as previously
Regards
Mike
> Kind regards
> Suzuki
>
>
> >
> > Regards
> >
> > Mike
> >
> >> Rob/Krzysztof,
> >>
> >> Do you have any recommendations for describing the 'hard wired
> >> ports' ?
> >>
> >> e.g:
> >>
> >> component {
> >> input_ports {
> >> component_input_port0: port@0 {
> >> ...
> >> <hard-wired-to*> = &component_output_port0;
> >> };
> >> ...
> >> };
> >>
> >> output_ports {
> >> componentne_output_port0: port@0 {
> >> ...
> >> <hard-wired-to> = &component_input_port0;
> >> };
> >> ...
> >> };
> >>
> >> };
> >>
> >> *Need a better suitable property than "hard-wired-to".
> >>
> >>
> >> Suzuki
> >>
> >>
> >>>
> >>>
> >>> Best,
> >>>
> >>> Tao
> >>>
> >>>>
> >>>>
> >>>>>
> >>>>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
> >>>>> what's the meaning
> >>>>
> >>>> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is acting
> >>>> like a cache for easing ATB data load, by providing h/w buffering.
> >>>> (In your case, it may not be providing any buffering, it doesn't matter
> >>>> either way, as it is not visible to the driver).
> >>>>
> >>>> Suzuki
> >>>>
> >>>>>
> >>>>> of this. Could you describe it in detail?
> >>>>>
> >>>>>
> >>>>> Best,
> >>>>>
> >>>>> Tao
> >>>>>
> >>>>>>
> >>>>>> Suzuki
> >>>>>>
> >>>>>>>
> >>>>>>> Signed-off-by: Tao Zhang <[email protected]>
> >>>>>>> ---
> >>>>>>> drivers/hwtracing/coresight/coresight-core.c | 81
> >>>>>>> ++++++++++++++++---
> >>>>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
> >>>>>>> include/linux/coresight.h | 2 +
> >>>>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
> >>>>>>> b/drivers/hwtracing/coresight/coresight-core.c
> >>>>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
> >>>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
> >>>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
> >>>>>>> @@ -113,15 +113,63 @@ struct coresight_device
> >>>>>>> *coresight_get_percpu_sink(int cpu)
> >>>>>>> }
> >>>>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
> >>>>>>> +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;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +/**
> >>>>>>> + * 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_label == NULL)
> >>>>>>> + return ret;
> >>>>>>> +
> >>>>>>> + source = coresight_get_source(path);
> >>>>>>> + if (source == NULL)
> >>>>>>> + return ret;
> >>>>>>> +
> >>>>>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >>>>>>> + conn->source_label))
> >>>>>>> + ret = 0;
> >>>>>>> + else
> >>>>>>> + ret = -1;
> >>>>>>> +
> >>>>>>> + return ret;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> static struct coresight_connection *
> >>>>>>> coresight_find_out_connection(struct coresight_device *src_dev,
> >>>>>>> - struct coresight_device *dest_dev)
> >>>>>>> + struct coresight_device *dest_dev,
> >>>>>>> + struct list_head *path)
> >>>>>>> {
> >>>>>>> int i;
> >>>>>>> struct coresight_connection *conn;
> >>>>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
> >>>>>>> conn = src_dev->pdata->out_conns[i];
> >>>>>>> + if (coresight_source_filter(path, conn))
> >>>>>>> + continue;
> >>>>>>> if (conn->dest_dev == dest_dev)
> >>>>>>> return conn;
> >>>>>>> }
> >>>>>>> @@ -312,7 +360,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;
> >>>>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
> >>>>>>> coresight_device *csdev,
> >>>>>>> if (!parent || !child)
> >>>>>>> return -EINVAL;
> >>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
> >>>>>>> - outconn = coresight_find_out_connection(csdev, child);
> >>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
> >>>>>>> + outconn = coresight_find_out_connection(csdev, child, path);
> >>>>>>> link_subtype = csdev->subtype.link_subtype;
> >>>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
> >>>>>>> IS_ERR(inconn))
> >>>>>>> @@ -341,7 +390,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;
> >>>>>>> int link_subtype;
> >>>>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
> >>>>>>> coresight_device *csdev,
> >>>>>>> if (!parent || !child)
> >>>>>>> return;
> >>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
> >>>>>>> - outconn = coresight_find_out_connection(csdev, child);
> >>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
> >>>>>>> + outconn = coresight_find_out_connection(csdev, child, path);
> >>>>>>> link_subtype = csdev->subtype.link_subtype;
> >>>>>>> if (link_ops(csdev)->disable) {
> >>>>>>> @@ -507,7 +557,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;
> >>>>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
> >>>>>>> *path, enum cs_mode mode,
> >>>>>>> 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;
> >>>>>>> @@ -802,7 +852,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;
> >>>>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
> >>>>>>> coresight_device *csdev,
> >>>>>>> if (coresight_is_percpu_source(csdev) &&
> >>>>>>> coresight_is_percpu_sink(sink) &&
> >>>>>>> sink == per_cpu(csdev_sink,
> >>>>>>> source_ops(csdev)->cpu_id(csdev))) {
> >>>>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
> >>>>>>> + if (_coresight_build_path(sink, sink, path, source) == 0) {
> >>>>>>> found = true;
> >>>>>>> goto out;
> >>>>>>> }
> >>>>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
> >>>>>>> coresight_device *csdev,
> >>>>>>> struct coresight_device *child_dev;
> >>>>>>> child_dev = csdev->pdata->out_conns[i]->dest_dev;
> >>>>>>> + if (csdev->pdata->out_conns[i]->source_label &&
> >>>>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >>>>>>> + csdev->pdata->out_conns[i]->source_label))
> >>>>>>> + continue;
> >>>>>>> if (child_dev &&
> >>>>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
> >>>>>>> + _coresight_build_path(child_dev, sink, path, source)
> >>>>>>> == 0) {
> >>>>>>> found = true;
> >>>>>>> break;
> >>>>>>> }
> >>>>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
> >>>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
> >>>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> >>>>>>> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
> >>>>>>> device *dev,
> >>>>>>> struct fwnode_handle *rdev_fwnode;
> >>>>>>> struct coresight_connection conn = {};
> >>>>>>> struct coresight_connection *new_conn;
> >>>>>>> + const char *label;
> >>>>>>> do {
> >>>>>>> /* Parse the local port details */
> >>>>>>> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
> >>>>>>> device *dev,
> >>>>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
> >>>>>>> conn.dest_port = rendpoint.port;
> >>>>>>> + conn.source_label = NULL;
> >>>>>>> + if (!of_property_read_string(ep, "label", &label))
> >>>>>>> + conn.source_label = label;
> >>>>>>> +
> >>>>>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
> >>>>>>> if (IS_ERR_VALUE(new_conn)) {
> >>>>>>> fwnode_handle_put(conn.dest_fwnode);
> >>>>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> >>>>>>> index e8b6e388218c..a9c06ef9bbb2 100644
> >>>>>>> --- a/include/linux/coresight.h
> >>>>>>> +++ b/include/linux/coresight.h
> >>>>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
> >>>>>>> * struct coresight_connection - representation of a single
> >>>>>>> connection
> >>>>>>> * @src_port: a connection's output port number.
> >>>>>>> * @dest_port: destination's input port number @src_port is
> >>>>>>> connected to.
> >>>>>>> + * @source_label: source component's label.
> >>>>>>> * @dest_fwnode: destination component's fwnode handle.
> >>>>>>> * @dest_dev: a @coresight_device representation of the component
> >>>>>>> connected to @src_port. NULL until the device is created
> >>>>>>> @@ -195,6 +196,7 @@ struct coresight_desc {
> >>>>>>> struct coresight_connection {
> >>>>>>> int src_port;
> >>>>>>> int dest_port;
> >>>>>>> + const char *source_label;
> >>>>>>> struct fwnode_handle *dest_fwnode;
> >>>>>>> struct coresight_device *dest_dev;
> >>>>>>> struct coresight_sysfs_link *link;
> >>>>>>
> >>>>
> >>
> >
> >
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Hi Mike
On 18/04/2024 09:48, Mike Leach wrote:
> Hi Suzuki
>
> On Wed, 17 Apr 2024 at 10:21, Suzuki K Poulose <[email protected]> wrote:
>>
>> Hi Mike
>>
>> On 16/04/2024 15:19, Mike Leach wrote:
>>> Hi,
>>>
>>> On Mon, 15 Apr 2024 at 14:24, Suzuki K Poulose <[email protected]> wrote:
>>>>
>>>> On 09/04/2024 14:22, Tao Zhang wrote:
>>>>>
>>>>> On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
>>>>>> Hi
>>>>>>
>>>>>> On 29/03/2024 09:27, Tao Zhang wrote:
>>>>>>>
>>>>>>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
>>>>>>>> On 21/03/2024 08:32, Tao Zhang wrote:
>>>>>>>>> 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
>>>>>>>>> port is selected according to the source in the trace path.
>>>>>>>>>
>>>>>>>>> The source of the input port on funnels will be marked in the
>>>>>>>>> device tree.
>>>>>>>>> e.g.
>>>>>>>>> tpdm@xxxxxxx {
>>>>>>>>> ... ... ... ...
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> funnel_XXX: funnel@xxxxxxx {
>>>>>>>>> ... ... ... ...
>>>>>>>>> out-ports {
>>>>>>>>> ... ... ... ...
>>>>>>>>> port@x {
>>>>>>>>> ... ... ... ...
>>>>>>>>> label = "xxxxxxx.tpdm"; <-- To label the source
>>>>>>>>> }; corresponding to the output
>>>>>>>>> ... ... ... ... connection "port@x". And this
>>>>>>>>> }; is a hardware static
>>>>>>>>> connections.
>>>>>>>>> ... ... ... ... Here needs to refer to hardware
>>>>>>>>> }; design.
>>>>>>>>>
>>>>>>>>> Then driver will parse the source label marked in the device tree, and
>>>>>>>>> save it to the coresight path. When the function needs to know the
>>>>>>>>> source label, it could obtain it from coresight path parameter.
>>>>>>>>> Finally,
>>>>>>>>> the output port knows which source it corresponds to, and it also
>>>>>>>>> knows
>>>>>>>>> which input port it corresponds to.
>>>>>>>>
>>>>>>>> Why do we need labels ? We have connection information for all devices
>>>>>>>> (both in and out), so, why do we need this label to find a device ?
>>>>>>>
>>>>>>> Because our funnel's design has multi-output ports, the data stream
>>>>>>> will not
>>>>>>>
>>>>>>> know which output port should pass in building the data trace path.
>>>>>>> This source
>>>>>>>
>>>>>>> label can make the data stream find the right output port to go.
>>>>>>>
>>>>>>>>
>>>>>>>> And also, I thought TPDM is a source device, why does a funnel output
>>>>>>>> port link to a source ?
>>>>>>>
>>>>>>> No, this label doesn't mean this funnel output port link to a source,
>>>>>>> it just let
>>>>>>>
>>>>>>> the output port know its data source.
>>>>>>>
>>>>>>>>
>>>>>>>> Are these funnels programmable ? Or, are they static ? If they are
>>>>>>>> static, do these need to be described in the DT ? If they are simply
>>>>>>>> acting as a "LINK" (or HWFIFO ?)
>>>>>>>
>>>>>>> These funnels are static, and we will add the "label" to the DT to
>>>>>>> describe the
>>>>>>>
>>>>>>> multi-output ports for these funnels.
>>>>>>
>>>>>> I think there is still a bit of confusion. By "Dynamic" I mean,
>>>>>> the "dynamic funnel" (explicit port enablement via MMIO) vs "static
>>>>>> funnel" (no programming, always ON).
>>>>>>
>>>>>> So, coming to your example, do we need to "explicitly" enable trace
>>>>>> flow for an "input" and/or an "output" port in your "funnel" ?
>>>>>
>>>>> Sorry for my misunderstanding in the previous mails. Our funnels are
>>>>> programmable just like the common dynamic funnels.
>>>>>
>>>>> In our solution, we just make funnels have multiple output ports
>>>>> connected to different devices or ports. When we use it, we still
>>>>>
>>>>> enable the input port through programming. Our solution is to know which
>>>>> input port the expected data comes from based on the
>>>>>
>>>>> source label corresponding to the output port. This way we can build the
>>>>> expected trace path. In other respects, it is used the same
>>>>>
>>>>> as common dynamic funnels.
>>>>
>>>>
>>>> Ok. So, to summarise :
>>>>
>>>> 1. This is not a standard Funnel, but a trace link with multiple-input
>>>> and multiple-output, with inputs hardwired to an outline at
>>>> integration.
>>>> 2. The programming model is same as that of a "standard funnel".
>>>>
>>>> Now, we do have enough information in the coresight_connections
>>>> to traverse input/output ports. But we need additional logic
>>>> to "hardwire" the ports to each other and necessary logic
>>>> to handle the
>>>>
>>>> There are two options here :
>>>>
>>>> 1. Treat this as a new component and have its own driver, with
>>>> additional logic to handle the input/output wiring.
>>>>
>>>> 2. Drive it using the funnel driver, with a a new compatible and
>>>> add additional logic to handle the input/output wiring.
>>>>
>>>> My inclination is towards (2), we need to see how this works out.
>>>>
>>>> We need to irrespective of the options, we need special handling
>>>> for hardwired ports in 1) building path 2) walking back the path
>>>> (in TPDA driver)
>>>>
>>>> We also need some "DT" information to bind a given input port
>>>> to an output port. We must not use "any device" labels to hack
>>>> this up, like the approach in this series.
>>>>
>>>
>>> Given that the internal connections are static for the given device,
>>> then the compatible will imply these connections in just the same way
>>> as the arm,coresight-funnel implies that all inputs are connected to
>>> the single output.
>>
>> I am sorry, I couldn't follow the last part. We have two or more output
>> ports and we need a way to identify, which input port is hardwired to
>> output-port0 and output-port1. Given we need special handling for these
>> anyway, I would like to avoid hard coding the input-output connection.
>> i.e., we do not want to assume that input-0 is always => output-0.
>>
>
> If we regard the current component as having compatible
> "qcom,coresight-compound-funnel-v1", then this identifies the
> relationship between the in-ports and out-ports.
> So the new driver / extension to the funnel driver that handles this
> compatible with know the static mapping between input and output so
> program it.
Ok, but like I said, having one compatible may not be enough to know
the "static" mapping for all possible device instances on different
SoCs.
>
> If however you want a more generic approach to handle future different
> versions of the component, then of course a method in DT of mapping
> in-ports to out-ports is useful.
>
> If did wonder if something along the lines of:-
>
> compound-funnel@0x1234000 {
> compatible = "compound-funnel"
> regs = < 0x1234000 0x1000>
> sub-funnel@0 {
> in-ports {
> [some port definitions]
> }
> out-ports {
> [some port definitions]
> }
> }
> sub-funnel@1 {
> in-ports {
> [some port definitions]
> }
> out-ports {
> [some port definitions]
> }
> }
> }
That would work, with "two" different coresight devices for each
"embedded funnel". And that also confuses the user with the topology.
>
>
> might be made to work? not sure about the implications of having more
> that one set of in-ports and out-ports in a component in the device
> tree & would need specific handling in the driver to iterate
> sub-funnels.
>
>
>>
>>>
>>> Irrespective of if a new driver is used, or an extension to the
>>> current funnel driver to handle a new compatible - the mapping between
>>> input and output ports are created based on the compatible..
>>>
>>> As we are building a path from source to sink, what is then needed is
>>> a method in the generic path building code, to recognise these
>>> amppings and filter the output ports that are searched based on the
>>> input port in use.
>>
>> Agreed. We could mark this as a property of the
>> port/coresight_connection.
>>
>>>
>>> On standard components, where the mapping is not present, then the
>>> code will continue as it does now, for these compound funnels, the
>>> mappings will be present and the output filtering will occur.
>>
>> Agreed
>>
>>> This removes the need for the labels / extra connection attributes on
>>> devices other than the funnel, and also removes the need to specify
>>> the internal connections as part of the device tree.
>>
>> I am still not clear how we map the input-output ports. Rest is what
>> exactly I had in mind. So, once we sort out the port mapping
>> we could proceed to the prototyping.
>>
>
> given we iterate by output port index into an array of out ports, and
> have an array of in-ports by index, a third mapping array, same size
> as in-ports, determining the associated out port index should suffice.
> Mapping array should be optional - if not there, path discovery works
> as previously
We could simply add a "(sticky)flag" to the
"coresight_connection".src_port/dest_port and extend the array to
include the sticky_port for src/dest port and use that flag to force
the path traversal.
Suzuki
>
> Regards
>
> Mike
>
>> Kind regards
>> Suzuki
>>
>>
>>>
>>> Regards
>>>
>>> Mike
>>>
>>>> Rob/Krzysztof,
>>>>
>>>> Do you have any recommendations for describing the 'hard wired
>>>> ports' ?
>>>>
>>>> e.g:
>>>>
>>>> component {
>>>> input_ports {
>>>> component_input_port0: port@0 {
>>>> ...
>>>> <hard-wired-to*> = &component_output_port0;
>>>> };
>>>> ...
>>>> };
>>>>
>>>> output_ports {
>>>> componentne_output_port0: port@0 {
>>>> ...
>>>> <hard-wired-to> = &component_input_port0;
>>>> };
>>>> ...
>>>> };
>>>>
>>>> };
>>>>
>>>> *Need a better suitable property than "hard-wired-to".
>>>>
>>>>
>>>> Suzuki
>>>>
>>>>
>>>>>
>>>>>
>>>>> Best,
>>>>>
>>>>> Tao
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
>>>>>>> what's the meaning
>>>>>>
>>>>>> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is acting
>>>>>> like a cache for easing ATB data load, by providing h/w buffering.
>>>>>> (In your case, it may not be providing any buffering, it doesn't matter
>>>>>> either way, as it is not visible to the driver).
>>>>>>
>>>>>> Suzuki
>>>>>>
>>>>>>>
>>>>>>> of this. Could you describe it in detail?
>>>>>>>
>>>>>>>
>>>>>>> Best,
>>>>>>>
>>>>>>> Tao
>>>>>>>
>>>>>>>>
>>>>>>>> Suzuki
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>>>> ---
>>>>>>>>> drivers/hwtracing/coresight/coresight-core.c | 81
>>>>>>>>> ++++++++++++++++---
>>>>>>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>>>>>>>>> include/linux/coresight.h | 2 +
>>>>>>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>> b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
>>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>> @@ -113,15 +113,63 @@ struct coresight_device
>>>>>>>>> *coresight_get_percpu_sink(int cpu)
>>>>>>>>> }
>>>>>>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>>>>>>>> +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;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +/**
>>>>>>>>> + * 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_label == NULL)
>>>>>>>>> + return ret;
>>>>>>>>> +
>>>>>>>>> + source = coresight_get_source(path);
>>>>>>>>> + if (source == NULL)
>>>>>>>>> + return ret;
>>>>>>>>> +
>>>>>>>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>>>>>> + conn->source_label))
>>>>>>>>> + ret = 0;
>>>>>>>>> + else
>>>>>>>>> + ret = -1;
>>>>>>>>> +
>>>>>>>>> + return ret;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> static struct coresight_connection *
>>>>>>>>> coresight_find_out_connection(struct coresight_device *src_dev,
>>>>>>>>> - struct coresight_device *dest_dev)
>>>>>>>>> + struct coresight_device *dest_dev,
>>>>>>>>> + struct list_head *path)
>>>>>>>>> {
>>>>>>>>> int i;
>>>>>>>>> struct coresight_connection *conn;
>>>>>>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>>>>>>> conn = src_dev->pdata->out_conns[i];
>>>>>>>>> + if (coresight_source_filter(path, conn))
>>>>>>>>> + continue;
>>>>>>>>> if (conn->dest_dev == dest_dev)
>>>>>>>>> return conn;
>>>>>>>>> }
>>>>>>>>> @@ -312,7 +360,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;
>>>>>>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>>>>>>>>> coresight_device *csdev,
>>>>>>>>> if (!parent || !child)
>>>>>>>>> return -EINVAL;
>>>>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>>>>>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>>>>>>>> IS_ERR(inconn))
>>>>>>>>> @@ -341,7 +390,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;
>>>>>>>>> int link_subtype;
>>>>>>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>>>>>>>>> coresight_device *csdev,
>>>>>>>>> if (!parent || !child)
>>>>>>>>> return;
>>>>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>>>>>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>>>> if (link_ops(csdev)->disable) {
>>>>>>>>> @@ -507,7 +557,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;
>>>>>>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
>>>>>>>>> *path, enum cs_mode mode,
>>>>>>>>> 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;
>>>>>>>>> @@ -802,7 +852,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;
>>>>>>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>>>>>>>>> coresight_device *csdev,
>>>>>>>>> if (coresight_is_percpu_source(csdev) &&
>>>>>>>>> coresight_is_percpu_sink(sink) &&
>>>>>>>>> sink == per_cpu(csdev_sink,
>>>>>>>>> source_ops(csdev)->cpu_id(csdev))) {
>>>>>>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
>>>>>>>>> + if (_coresight_build_path(sink, sink, path, source) == 0) {
>>>>>>>>> found = true;
>>>>>>>>> goto out;
>>>>>>>>> }
>>>>>>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>>>>>>>>> coresight_device *csdev,
>>>>>>>>> struct coresight_device *child_dev;
>>>>>>>>> child_dev = csdev->pdata->out_conns[i]->dest_dev;
>>>>>>>>> + if (csdev->pdata->out_conns[i]->source_label &&
>>>>>>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>>>>>> + csdev->pdata->out_conns[i]->source_label))
>>>>>>>>> + continue;
>>>>>>>>> if (child_dev &&
>>>>>>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>>>>>>>> + _coresight_build_path(child_dev, sink, path, source)
>>>>>>>>> == 0) {
>>>>>>>>> found = true;
>>>>>>>>> break;
>>>>>>>>> }
>>>>>>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
>>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>>>> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
>>>>>>>>> device *dev,
>>>>>>>>> struct fwnode_handle *rdev_fwnode;
>>>>>>>>> struct coresight_connection conn = {};
>>>>>>>>> struct coresight_connection *new_conn;
>>>>>>>>> + const char *label;
>>>>>>>>> do {
>>>>>>>>> /* Parse the local port details */
>>>>>>>>> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
>>>>>>>>> device *dev,
>>>>>>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>>>>>>>> conn.dest_port = rendpoint.port;
>>>>>>>>> + conn.source_label = NULL;
>>>>>>>>> + if (!of_property_read_string(ep, "label", &label))
>>>>>>>>> + conn.source_label = label;
>>>>>>>>> +
>>>>>>>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
>>>>>>>>> if (IS_ERR_VALUE(new_conn)) {
>>>>>>>>> fwnode_handle_put(conn.dest_fwnode);
>>>>>>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>>>>>>>> index e8b6e388218c..a9c06ef9bbb2 100644
>>>>>>>>> --- a/include/linux/coresight.h
>>>>>>>>> +++ b/include/linux/coresight.h
>>>>>>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
>>>>>>>>> * struct coresight_connection - representation of a single
>>>>>>>>> connection
>>>>>>>>> * @src_port: a connection's output port number.
>>>>>>>>> * @dest_port: destination's input port number @src_port is
>>>>>>>>> connected to.
>>>>>>>>> + * @source_label: source component's label.
>>>>>>>>> * @dest_fwnode: destination component's fwnode handle.
>>>>>>>>> * @dest_dev: a @coresight_device representation of the component
>>>>>>>>> connected to @src_port. NULL until the device is created
>>>>>>>>> @@ -195,6 +196,7 @@ struct coresight_desc {
>>>>>>>>> struct coresight_connection {
>>>>>>>>> int src_port;
>>>>>>>>> int dest_port;
>>>>>>>>> + const char *source_label;
>>>>>>>>> struct fwnode_handle *dest_fwnode;
>>>>>>>>> struct coresight_device *dest_dev;
>>>>>>>>> struct coresight_sysfs_link *link;
>>>>>>>>
>>>>>>
>>>>
>>>
>>>
>>
>
>
Hi Suzuki
> -----Original Message-----
> From: Suzuki K Poulose <[email protected]>
> Sent: Thursday, April 18, 2024 10:00 AM
> To: Mike Leach <[email protected]>
> Cc: Tao Zhang <[email protected]>; Mathieu Poirier
> <[email protected]>; Alexander Shishkin
> <[email protected]>; Konrad Dybcio
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
> Kozlowski <[email protected]>; Jinlong Mao
> <[email protected]>; Greg Kroah-Hartman
> <[email protected]>; [email protected]; linux-arm-
> [email protected]; [email protected];
> [email protected]; Tingwei Zhang <[email protected]>;
> Yuanfang Zhang <[email protected]>; Trilok Soni
> <[email protected]>; Song Chai <[email protected]>; linux-arm-
> [email protected]; [email protected]
> Subject: Re: [PATCH 2/4] coresight: Add support for multiple output ports on the
> funnel
>
> Hi Mike
>
> On 18/04/2024 09:48, Mike Leach wrote:
> > Hi Suzuki
> >
> > On Wed, 17 Apr 2024 at 10:21, Suzuki K Poulose <[email protected]>
> wrote:
> >>
> >> Hi Mike
> >>
> >> On 16/04/2024 15:19, Mike Leach wrote:
> >>> Hi,
> >>>
> >>> On Mon, 15 Apr 2024 at 14:24, Suzuki K Poulose <[email protected]>
> wrote:
> >>>>
> >>>> On 09/04/2024 14:22, Tao Zhang wrote:
> >>>>>
> >>>>> On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
> >>>>>> Hi
> >>>>>>
> >>>>>> On 29/03/2024 09:27, Tao Zhang wrote:
> >>>>>>>
> >>>>>>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
> >>>>>>>> On 21/03/2024 08:32, Tao Zhang wrote:
> >>>>>>>>> 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 port is selected according to the source in the trace
> path.
> >>>>>>>>>
> >>>>>>>>> The source of the input port on funnels will be marked in the
> >>>>>>>>> device tree.
> >>>>>>>>> e.g.
> >>>>>>>>> tpdm@xxxxxxx {
> >>>>>>>>> ... ... ... ...
> >>>>>>>>> };
> >>>>>>>>>
> >>>>>>>>> funnel_XXX: funnel@xxxxxxx {
> >>>>>>>>> ... ... ... ...
> >>>>>>>>> out-ports {
> >>>>>>>>> ... ... ... ...
> >>>>>>>>> port@x {
> >>>>>>>>> ... ... ... ...
> >>>>>>>>> label = "xxxxxxx.tpdm"; <-- To label the source
> >>>>>>>>> }; corresponding to the output
> >>>>>>>>> ... ... ... ... connection "port@x". And this
> >>>>>>>>> }; is a hardware static
> >>>>>>>>> connections.
> >>>>>>>>> ... ... ... ... Here needs to refer to hardware
> >>>>>>>>> }; design.
> >>>>>>>>>
> >>>>>>>>> Then driver will parse the source label marked in the device
> >>>>>>>>> tree, and save it to the coresight path. When the function
> >>>>>>>>> needs to know the source label, it could obtain it from coresight path
> parameter.
> >>>>>>>>> Finally,
> >>>>>>>>> the output port knows which source it corresponds to, and it
> >>>>>>>>> also knows which input port it corresponds to.
> >>>>>>>>
> >>>>>>>> Why do we need labels ? We have connection information for all
> >>>>>>>> devices (both in and out), so, why do we need this label to find a device
> ?
> >>>>>>>
> >>>>>>> Because our funnel's design has multi-output ports, the data
> >>>>>>> stream will not
> >>>>>>>
> >>>>>>> know which output port should pass in building the data trace path.
> >>>>>>> This source
> >>>>>>>
> >>>>>>> label can make the data stream find the right output port to go.
> >>>>>>>
> >>>>>>>>
> >>>>>>>> And also, I thought TPDM is a source device, why does a funnel
> >>>>>>>> output port link to a source ?
> >>>>>>>
> >>>>>>> No, this label doesn't mean this funnel output port link to a
> >>>>>>> source, it just let
> >>>>>>>
> >>>>>>> the output port know its data source.
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Are these funnels programmable ? Or, are they static ? If they
> >>>>>>>> are static, do these need to be described in the DT ? If they
> >>>>>>>> are simply acting as a "LINK" (or HWFIFO ?)
> >>>>>>>
> >>>>>>> These funnels are static, and we will add the "label" to the DT
> >>>>>>> to describe the
> >>>>>>>
> >>>>>>> multi-output ports for these funnels.
> >>>>>>
> >>>>>> I think there is still a bit of confusion. By "Dynamic" I mean,
> >>>>>> the "dynamic funnel" (explicit port enablement via MMIO) vs
> >>>>>> "static funnel" (no programming, always ON).
> >>>>>>
> >>>>>> So, coming to your example, do we need to "explicitly" enable
> >>>>>> trace flow for an "input" and/or an "output" port in your "funnel" ?
> >>>>>
> >>>>> Sorry for my misunderstanding in the previous mails. Our funnels
> >>>>> are programmable just like the common dynamic funnels.
> >>>>>
> >>>>> In our solution, we just make funnels have multiple output ports
> >>>>> connected to different devices or ports. When we use it, we still
> >>>>>
> >>>>> enable the input port through programming. Our solution is to know
> >>>>> which input port the expected data comes from based on the
> >>>>>
> >>>>> source label corresponding to the output port. This way we can
> >>>>> build the expected trace path. In other respects, it is used the
> >>>>> same
> >>>>>
> >>>>> as common dynamic funnels.
> >>>>
> >>>>
> >>>> Ok. So, to summarise :
> >>>>
> >>>> 1. This is not a standard Funnel, but a trace link with multiple-input
> >>>> and multiple-output, with inputs hardwired to an outline at
> >>>> integration.
> >>>> 2. The programming model is same as that of a "standard funnel".
> >>>>
> >>>> Now, we do have enough information in the coresight_connections to
> >>>> traverse input/output ports. But we need additional logic to
> >>>> "hardwire" the ports to each other and necessary logic to handle
> >>>> the
> >>>>
> >>>> There are two options here :
> >>>>
> >>>> 1. Treat this as a new component and have its own driver, with
> >>>> additional logic to handle the input/output wiring.
> >>>>
> >>>> 2. Drive it using the funnel driver, with a a new compatible and
> >>>> add additional logic to handle the input/output wiring.
> >>>>
> >>>> My inclination is towards (2), we need to see how this works out.
> >>>>
> >>>> We need to irrespective of the options, we need special handling
> >>>> for hardwired ports in 1) building path 2) walking back the path
> >>>> (in TPDA driver)
> >>>>
> >>>> We also need some "DT" information to bind a given input port to an
> >>>> output port. We must not use "any device" labels to hack this up,
> >>>> like the approach in this series.
> >>>>
> >>>
> >>> Given that the internal connections are static for the given device,
> >>> then the compatible will imply these connections in just the same
> >>> way as the arm,coresight-funnel implies that all inputs are
> >>> connected to the single output.
> >>
> >> I am sorry, I couldn't follow the last part. We have two or more
> >> output ports and we need a way to identify, which input port is
> >> hardwired to
> >> output-port0 and output-port1. Given we need special handling for
> >> these anyway, I would like to avoid hard coding the input-output connection.
> >> i.e., we do not want to assume that input-0 is always => output-0.
> >>
> >
> > If we regard the current component as having compatible
> > "qcom,coresight-compound-funnel-v1", then this identifies the
> > relationship between the in-ports and out-ports.
> > So the new driver / extension to the funnel driver that handles this
> > compatible with know the static mapping between input and output so
> > program it.
>
> Ok, but like I said, having one compatible may not be enough to know the "static"
> mapping for all possible device instances on different SoCs.
>
The compatible name would have to change if the mapping changed.
Using the compatible is simpler, but less flexible
> >
> > If however you want a more generic approach to handle future different
> > versions of the component, then of course a method in DT of mapping
> > in-ports to out-ports is useful.
> >
> > If did wonder if something along the lines of:-
> >
> > compound-funnel@0x1234000 {
> > compatible = "compound-funnel"
> > regs = < 0x1234000 0x1000>
> > sub-funnel@0 {
> > in-ports {
> > [some port definitions]
> > }
> > out-ports {
> > [some port definitions]
> > }
> > }
> > sub-funnel@1 {
> > in-ports {
> > [some port definitions]
> > }
> > out-ports {
> > [some port definitions]
> > }
> > }
> > }
>
> That would work, with "two" different coresight devices for each "embedded
> funnel". And that also confuses the user with the topology.
I wasn't suggesting two different coresight devices, but finding a way to iterate the sub-nodes to create a single device with the inputs mapped to outputs. Which may or may not be easily do-able.
As to topology - no more confusing than a "funnel" with multiple outputs, or phandle links between inputs and outputs. It does visually represent what the device really is - multiple effective funnels controlled by a single set of registers.
Mike
>
> >
> >
> > might be made to work? not sure about the implications of having more
> > that one set of in-ports and out-ports in a component in the device
> > tree & would need specific handling in the driver to iterate
> > sub-funnels.
> >
> >
> >>
> >>>
> >>> Irrespective of if a new driver is used, or an extension to the
> >>> current funnel driver to handle a new compatible - the mapping
> >>> between input and output ports are created based on the compatible..
> >>>
> >>> As we are building a path from source to sink, what is then needed
> >>> is a method in the generic path building code, to recognise these
> >>> amppings and filter the output ports that are searched based on the
> >>> input port in use.
> >>
> >> Agreed. We could mark this as a property of the
> >> port/coresight_connection.
> >>
> >>>
> >>> On standard components, where the mapping is not present, then the
> >>> code will continue as it does now, for these compound funnels, the
> >>> mappings will be present and the output filtering will occur.
> >>
> >> Agreed
> >>
> >>> This removes the need for the labels / extra connection attributes
> >>> on devices other than the funnel, and also removes the need to
> >>> specify the internal connections as part of the device tree.
> >>
> >> I am still not clear how we map the input-output ports. Rest is what
> >> exactly I had in mind. So, once we sort out the port mapping we could
> >> proceed to the prototyping.
> >>
> >
> > given we iterate by output port index into an array of out ports, and
> > have an array of in-ports by index, a third mapping array, same size
> > as in-ports, determining the associated out port index should suffice.
> > Mapping array should be optional - if not there, path discovery works
> > as previously
>
> We could simply add a "(sticky)flag" to the
> "coresight_connection".src_port/dest_port and extend the array to include the
> sticky_port for src/dest port and use that flag to force the path traversal.
>
> Suzuki
>
>
>
> >
> > Regards
> >
> > Mike
> >
> >> Kind regards
> >> Suzuki
> >>
> >>
> >>>
> >>> Regards
> >>>
> >>> Mike
> >>>
> >>>> Rob/Krzysztof,
> >>>>
> >>>> Do you have any recommendations for describing the 'hard wired
> >>>> ports' ?
> >>>>
> >>>> e.g:
> >>>>
> >>>> component {
> >>>> input_ports {
> >>>> component_input_port0: port@0 {
> >>>> ...
> >>>> <hard-wired-to*> = &component_output_port0;
> >>>> };
> >>>> ...
> >>>> };
> >>>>
> >>>> output_ports {
> >>>> componentne_output_port0: port@0 {
> >>>> ...
> >>>> <hard-wired-to> = &component_input_port0;
> >>>> };
> >>>> ...
> >>>> };
> >>>>
> >>>> };
> >>>>
> >>>> *Need a better suitable property than "hard-wired-to".
> >>>>
> >>>>
> >>>> Suzuki
> >>>>
> >>>>
> >>>>>
> >>>>>
> >>>>> Best,
> >>>>>
> >>>>> Tao
> >>>>>
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not
> >>>>>>> sure what's the meaning
> >>>>>>
> >>>>>> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is
> >>>>>> acting like a cache for easing ATB data load, by providing h/w buffering.
> >>>>>> (In your case, it may not be providing any buffering, it doesn't
> >>>>>> matter either way, as it is not visible to the driver).
> >>>>>>
> >>>>>> Suzuki
> >>>>>>
> >>>>>>>
> >>>>>>> of this. Could you describe it in detail?
> >>>>>>>
> >>>>>>>
> >>>>>>> Best,
> >>>>>>>
> >>>>>>> Tao
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Suzuki
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
> >>>>>>>>> ---
> >>>>>>>>> drivers/hwtracing/coresight/coresight-core.c | 81
> >>>>>>>>> ++++++++++++++++---
> >>>>>>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
> >>>>>>>>> include/linux/coresight.h | 2 +
> >>>>>>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
> >>>>>>>>> b/drivers/hwtracing/coresight/coresight-core.c
> >>>>>>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
> >>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
> >>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
> >>>>>>>>> @@ -113,15 +113,63 @@ struct coresight_device
> >>>>>>>>> *coresight_get_percpu_sink(int cpu)
> >>>>>>>>> }
> >>>>>>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
> >>>>>>>>> +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;
> >>>>>>>>> +}
> >>>>>>>>> +
> >>>>>>>>> +/**
> >>>>>>>>> + * 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_label == NULL)
> >>>>>>>>> + return ret;
> >>>>>>>>> +
> >>>>>>>>> + source = coresight_get_source(path);
> >>>>>>>>> + if (source == NULL)
> >>>>>>>>> + return ret;
> >>>>>>>>> +
> >>>>>>>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >>>>>>>>> + conn->source_label))
> >>>>>>>>> + ret = 0;
> >>>>>>>>> + else
> >>>>>>>>> + ret = -1;
> >>>>>>>>> +
> >>>>>>>>> + return ret;
> >>>>>>>>> +}
> >>>>>>>>> +
> >>>>>>>>> static struct coresight_connection *
> >>>>>>>>> coresight_find_out_connection(struct coresight_device *src_dev,
> >>>>>>>>> - struct coresight_device *dest_dev)
> >>>>>>>>> + struct coresight_device *dest_dev,
> >>>>>>>>> + struct list_head *path)
> >>>>>>>>> {
> >>>>>>>>> int i;
> >>>>>>>>> struct coresight_connection *conn;
> >>>>>>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
> >>>>>>>>> conn = src_dev->pdata->out_conns[i];
> >>>>>>>>> + if (coresight_source_filter(path, conn))
> >>>>>>>>> + continue;
> >>>>>>>>> if (conn->dest_dev == dest_dev)
> >>>>>>>>> return conn;
> >>>>>>>>> }
> >>>>>>>>> @@ -312,7 +360,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;
> >>>>>>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
> >>>>>>>>> coresight_device *csdev,
> >>>>>>>>> if (!parent || !child)
> >>>>>>>>> return -EINVAL;
> >>>>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
> >>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
> >>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
> >>>>>>>>> + outconn = coresight_find_out_connection(csdev, child,
> >>>>>>>>> + path);
> >>>>>>>>> link_subtype = csdev->subtype.link_subtype;
> >>>>>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG
> >>>>>>>>> &&
> >>>>>>>>> IS_ERR(inconn))
> >>>>>>>>> @@ -341,7 +390,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;
> >>>>>>>>> int link_subtype;
> >>>>>>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
> >>>>>>>>> coresight_device *csdev,
> >>>>>>>>> if (!parent || !child)
> >>>>>>>>> return;
> >>>>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
> >>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
> >>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
> >>>>>>>>> + outconn = coresight_find_out_connection(csdev, child,
> >>>>>>>>> + path);
> >>>>>>>>> link_subtype = csdev->subtype.link_subtype;
> >>>>>>>>> if (link_ops(csdev)->disable) { @@ -507,7 +557,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;
> >>>>>>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
> >>>>>>>>> *path, enum cs_mode mode,
> >>>>>>>>> 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;
> >>>>>>>>> @@ -802,7 +852,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;
> >>>>>>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
> >>>>>>>>> coresight_device *csdev,
> >>>>>>>>> if (coresight_is_percpu_source(csdev) &&
> >>>>>>>>> coresight_is_percpu_sink(sink) &&
> >>>>>>>>> sink == per_cpu(csdev_sink,
> >>>>>>>>> source_ops(csdev)->cpu_id(csdev))) {
> >>>>>>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
> >>>>>>>>> + if (_coresight_build_path(sink, sink, path, source)
> >>>>>>>>> + == 0) {
> >>>>>>>>> found = true;
> >>>>>>>>> goto out;
> >>>>>>>>> }
> >>>>>>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
> >>>>>>>>> coresight_device *csdev,
> >>>>>>>>> struct coresight_device *child_dev;
> >>>>>>>>> child_dev =
> >>>>>>>>> csdev->pdata->out_conns[i]->dest_dev;
> >>>>>>>>> + if (csdev->pdata->out_conns[i]->source_label &&
> >>>>>>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >>>>>>>>> + csdev->pdata->out_conns[i]->source_label))
> >>>>>>>>> + continue;
> >>>>>>>>> if (child_dev &&
> >>>>>>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
> >>>>>>>>> + _coresight_build_path(child_dev, sink, path,
> >>>>>>>>> + source)
> >>>>>>>>> == 0) {
> >>>>>>>>> found = true;
> >>>>>>>>> break;
> >>>>>>>>> }
> >>>>>>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
> >>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
> >>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> >>>>>>>>> @@ -205,6 +205,7 @@ static int
> >>>>>>>>> of_coresight_parse_endpoint(struct
> >>>>>>>>> device *dev,
> >>>>>>>>> struct fwnode_handle *rdev_fwnode;
> >>>>>>>>> struct coresight_connection conn = {};
> >>>>>>>>> struct coresight_connection *new_conn;
> >>>>>>>>> + const char *label;
> >>>>>>>>> do {
> >>>>>>>>> /* Parse the local port details */ @@ -243,6
> >>>>>>>>> +244,10 @@ static int of_coresight_parse_endpoint(struct
> >>>>>>>>> device *dev,
> >>>>>>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
> >>>>>>>>> conn.dest_port = rendpoint.port;
> >>>>>>>>> + conn.source_label = NULL;
> >>>>>>>>> + if (!of_property_read_string(ep, "label", &label))
> >>>>>>>>> + conn.source_label = label;
> >>>>>>>>> +
> >>>>>>>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
> >>>>>>>>> if (IS_ERR_VALUE(new_conn)) {
> >>>>>>>>> fwnode_handle_put(conn.dest_fwnode);
> >>>>>>>>> diff --git a/include/linux/coresight.h
> >>>>>>>>> b/include/linux/coresight.h index e8b6e388218c..a9c06ef9bbb2
> >>>>>>>>> 100644
> >>>>>>>>> --- a/include/linux/coresight.h
> >>>>>>>>> +++ b/include/linux/coresight.h
> >>>>>>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
> >>>>>>>>> * struct coresight_connection - representation of a
> >>>>>>>>> single connection
> >>>>>>>>> * @src_port: a connection's output port number.
> >>>>>>>>> * @dest_port: destination's input port number @src_port is
> >>>>>>>>> connected to.
> >>>>>>>>> + * @source_label: source component's label.
> >>>>>>>>> * @dest_fwnode: destination component's fwnode handle.
> >>>>>>>>> * @dest_dev: a @coresight_device representation of the
> component
> >>>>>>>>> connected to @src_port. NULL until the device is
> >>>>>>>>> created @@ -195,6 +196,7 @@ struct coresight_desc {
> >>>>>>>>> struct coresight_connection {
> >>>>>>>>> int src_port;
> >>>>>>>>> int dest_port;
> >>>>>>>>> + const char *source_label;
> >>>>>>>>> struct fwnode_handle *dest_fwnode;
> >>>>>>>>> struct coresight_device *dest_dev;
> >>>>>>>>> struct coresight_sysfs_link *link;
> >>>>>>>>
> >>>>>>
> >>>>
> >>>
> >>>
> >>
> >
> >
>
> _______________________________________________
> CoreSight mailing list -- [email protected] To unsubscribe send an email to
> [email protected]
Hi Mike and Suzuki
On 4/18/2024 5:25 PM, Mike Leach wrote:
> Hi Suzuki
>
>> -----Original Message-----
>> From: Suzuki K Poulose <[email protected]>
>> Sent: Thursday, April 18, 2024 10:00 AM
>> To: Mike Leach <[email protected]>
>> Cc: Tao Zhang <[email protected]>; Mathieu Poirier
>> <[email protected]>; Alexander Shishkin
>> <[email protected]>; Konrad Dybcio
>> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
>> Kozlowski <[email protected]>; Jinlong Mao
>> <[email protected]>; Greg Kroah-Hartman
>> <[email protected]>; [email protected]; linux-arm-
>> [email protected]; [email protected];
>> [email protected]; Tingwei Zhang <[email protected]>;
>> Yuanfang Zhang <[email protected]>; Trilok Soni
>> <[email protected]>; Song Chai <[email protected]>; linux-arm-
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 2/4] coresight: Add support for multiple output ports on the
>> funnel
>>
>> Hi Mike
>>
>> On 18/04/2024 09:48, Mike Leach wrote:
>>> Hi Suzuki
>>>
>>> On Wed, 17 Apr 2024 at 10:21, Suzuki K Poulose <[email protected]>
>> wrote:
>>>>
>>>> Hi Mike
>>>>
>>>> On 16/04/2024 15:19, Mike Leach wrote:
>>>>> Hi,
>>>>>
>>>>> On Mon, 15 Apr 2024 at 14:24, Suzuki K Poulose <[email protected]>
>> wrote:
>>>>>>
>>>>>> On 09/04/2024 14:22, Tao Zhang wrote:
>>>>>>>
>>>>>>> On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On 29/03/2024 09:27, Tao Zhang wrote:
>>>>>>>>>
>>>>>>>>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
>>>>>>>>>> On 21/03/2024 08:32, Tao Zhang wrote:
>>>>>>>>>>> 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 port is selected according to the source in the trace
>> path.
>>>>>>>>>>>
>>>>>>>>>>> The source of the input port on funnels will be marked in the
>>>>>>>>>>> device tree.
>>>>>>>>>>> e.g.
>>>>>>>>>>> tpdm@xxxxxxx {
>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>> };
>>>>>>>>>>>
>>>>>>>>>>> funnel_XXX: funnel@xxxxxxx {
>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>> out-ports {
>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>> port@x {
>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>> label = "xxxxxxx.tpdm"; <-- To label the source
>>>>>>>>>>> }; corresponding to the output
>>>>>>>>>>> ... ... ... ... connection "port@x". And this
>>>>>>>>>>> }; is a hardware static
>>>>>>>>>>> connections.
>>>>>>>>>>> ... ... ... ... Here needs to refer to hardware
>>>>>>>>>>> }; design.
>>>>>>>>>>>
>>>>>>>>>>> Then driver will parse the source label marked in the device
>>>>>>>>>>> tree, and save it to the coresight path. When the function
>>>>>>>>>>> needs to know the source label, it could obtain it from coresight path
>> parameter.
>>>>>>>>>>> Finally,
>>>>>>>>>>> the output port knows which source it corresponds to, and it
>>>>>>>>>>> also knows which input port it corresponds to.
>>>>>>>>>>
>>>>>>>>>> Why do we need labels ? We have connection information for all
>>>>>>>>>> devices (both in and out), so, why do we need this label to find a device
>> ?
>>>>>>>>>
>>>>>>>>> Because our funnel's design has multi-output ports, the data
>>>>>>>>> stream will not
>>>>>>>>>
>>>>>>>>> know which output port should pass in building the data trace path.
>>>>>>>>> This source
>>>>>>>>>
>>>>>>>>> label can make the data stream find the right output port to go.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> And also, I thought TPDM is a source device, why does a funnel
>>>>>>>>>> output port link to a source ?
>>>>>>>>>
>>>>>>>>> No, this label doesn't mean this funnel output port link to a
>>>>>>>>> source, it just let
>>>>>>>>>
>>>>>>>>> the output port know its data source.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Are these funnels programmable ? Or, are they static ? If they
>>>>>>>>>> are static, do these need to be described in the DT ? If they
>>>>>>>>>> are simply acting as a "LINK" (or HWFIFO ?)
>>>>>>>>>
>>>>>>>>> These funnels are static, and we will add the "label" to the DT
>>>>>>>>> to describe the
>>>>>>>>>
>>>>>>>>> multi-output ports for these funnels.
>>>>>>>>
>>>>>>>> I think there is still a bit of confusion. By "Dynamic" I mean,
>>>>>>>> the "dynamic funnel" (explicit port enablement via MMIO) vs
>>>>>>>> "static funnel" (no programming, always ON).
>>>>>>>>
>>>>>>>> So, coming to your example, do we need to "explicitly" enable
>>>>>>>> trace flow for an "input" and/or an "output" port in your "funnel" ?
>>>>>>>
>>>>>>> Sorry for my misunderstanding in the previous mails. Our funnels
>>>>>>> are programmable just like the common dynamic funnels.
>>>>>>>
>>>>>>> In our solution, we just make funnels have multiple output ports
>>>>>>> connected to different devices or ports. When we use it, we still
>>>>>>>
>>>>>>> enable the input port through programming. Our solution is to know
>>>>>>> which input port the expected data comes from based on the
>>>>>>>
>>>>>>> source label corresponding to the output port. This way we can
>>>>>>> build the expected trace path. In other respects, it is used the
>>>>>>> same
>>>>>>>
>>>>>>> as common dynamic funnels.
>>>>>>
>>>>>>
>>>>>> Ok. So, to summarise :
>>>>>>
>>>>>> 1. This is not a standard Funnel, but a trace link with multiple-input
>>>>>> and multiple-output, with inputs hardwired to an outline at
>>>>>> integration.
>>>>>> 2. The programming model is same as that of a "standard funnel".
>>>>>>
>>>>>> Now, we do have enough information in the coresight_connections to
>>>>>> traverse input/output ports. But we need additional logic to
>>>>>> "hardwire" the ports to each other and necessary logic to handle
>>>>>> the
>>>>>>
>>>>>> There are two options here :
>>>>>>
>>>>>> 1. Treat this as a new component and have its own driver, with
>>>>>> additional logic to handle the input/output wiring.
>>>>>>
>>>>>> 2. Drive it using the funnel driver, with a a new compatible and
>>>>>> add additional logic to handle the input/output wiring.
>>>>>>
>>>>>> My inclination is towards (2), we need to see how this works out.
>>>>>>
>>>>>> We need to irrespective of the options, we need special handling
>>>>>> for hardwired ports in 1) building path 2) walking back the path
>>>>>> (in TPDA driver)
>>>>>>
>>>>>> We also need some "DT" information to bind a given input port to an
>>>>>> output port. We must not use "any device" labels to hack this up,
>>>>>> like the approach in this series.
>>>>>>
>>>>>
>>>>> Given that the internal connections are static for the given device,
>>>>> then the compatible will imply these connections in just the same
>>>>> way as the arm,coresight-funnel implies that all inputs are
>>>>> connected to the single output.
>>>>
>>>> I am sorry, I couldn't follow the last part. We have two or more
>>>> output ports and we need a way to identify, which input port is
>>>> hardwired to
>>>> output-port0 and output-port1. Given we need special handling for
>>>> these anyway, I would like to avoid hard coding the input-output connection.
>>>> i.e., we do not want to assume that input-0 is always => output-0.
>>>>
>>>
>>> If we regard the current component as having compatible
>>> "qcom,coresight-compound-funnel-v1", then this identifies the
>>> relationship between the in-ports and out-ports.
>>> So the new driver / extension to the funnel driver that handles this
>>> compatible with know the static mapping between input and output so
>>> program it.
>>
>> Ok, but like I said, having one compatible may not be enough to know the "static"
>> mapping for all possible device instances on different SoCs.
>>
>
> The compatible name would have to change if the mapping changed.
> Using the compatible is simpler, but less flexible
>
>>>
>>> If however you want a more generic approach to handle future different
>>> versions of the component, then of course a method in DT of mapping
>>> in-ports to out-ports is useful.
>>>
>>> If did wonder if something along the lines of:-
>>>
>>> compound-funnel@0x1234000 {
>>> compatible = "compound-funnel"
>>> regs = < 0x1234000 0x1000>
>>> sub-funnel@0 {
>>> in-ports {
>>> [some port definitions]
>>> }
>>> out-ports {
>>> [some port definitions]
>>> }
>>> }
>>> sub-funnel@1 {
>>> in-ports {
>>> [some port definitions]
>>> }
>>> out-ports {
>>> [some port definitions]
>>> }
>>> }
>>> }
>>
>> That would work, with "two" different coresight devices for each "embedded
>> funnel". And that also confuses the user with the topology.
>
> I wasn't suggesting two different coresight devices, but finding a way to iterate the sub-nodes to create a single device with the inputs mapped to outputs. Which may or may not be easily do-able.
>
> As to topology - no more confusing than a "funnel" with multiple outputs, or phandle links between inputs and outputs. It does visually represent what the device really is - multiple effective funnels controlled by a single set of registers.
>
Let me provide the hardware topology here to facilitate the discussion.
|----------| |---------| |----------| |---------|
| TPDM 0 | | Source0 | | Source 1 | | TPDM 1 |
|----------| |---------| |----------| |---------|
| | | |
| | | |
| --------- | | |
| | | |
| | | |
| | | |
\-------------/ ---------------------- |
\ Funnel 0 / | |
----------- | ---------------------------------
| | |
| | |
\ -------------/
\ Funnel 1 /
-----------/
| |---------------------
| |------------ |
| |TPDM0 |TPDM1
| \ ----------------/
| \ TPDA 0 /
| --------------/
| |
| |
|Source0/1 |
\-------------------------------/
\ Funnel 2 /
---------------------------
To describe this topology in device tree, we need to indicate input
port0 of FUNNEL0 is static link to output port0 of FUNNEL0 which links
to input port0 of TPDA0. When code builds the path, it can get the
static link information from topology and select correct path. As Suzuki
has suggested, we can describe the topology like below.
funnel0 {
...
in-ports {
port@0 {
funnel0_in0: endpoint {
remote-endpoint = <&tpdm0_output>;
<hard-wired-to*>=
<&funnel1_out0>;
}
}
}
}
funnel1 {
...
out-ports {
port@0 {
funnel1_out0: endpoint {
remote-endpoint= <&tpdm0_in0>;
<hard-wired-to*>=
<&funnel0_in0>;
}
}
}
}
> Mike
>
>>
>>>
>>>
>>> might be made to work? not sure about the implications of having more
>>> that one set of in-ports and out-ports in a component in the device
>>> tree & would need specific handling in the driver to iterate
>>> sub-funnels.
>>>
>>>
>>>>
>>>>>
>>>>> Irrespective of if a new driver is used, or an extension to the
>>>>> current funnel driver to handle a new compatible - the mapping
>>>>> between input and output ports are created based on the compatible..
>>>>>
>>>>> As we are building a path from source to sink, what is then needed
>>>>> is a method in the generic path building code, to recognise these
>>>>> amppings and filter the output ports that are searched based on the
>>>>> input port in use.
>>>>
>>>> Agreed. We could mark this as a property of the
>>>> port/coresight_connection.
>>>>
>>>>>
>>>>> On standard components, where the mapping is not present, then the
>>>>> code will continue as it does now, for these compound funnels, the
>>>>> mappings will be present and the output filtering will occur.
>>>>
>>>> Agreed
>>>>
>>>>> This removes the need for the labels / extra connection attributes
>>>>> on devices other than the funnel, and also removes the need to
>>>>> specify the internal connections as part of the device tree.
>>>>
>>>> I am still not clear how we map the input-output ports. Rest is what
>>>> exactly I had in mind. So, once we sort out the port mapping we could
>>>> proceed to the prototyping.
>>>>
>>>
>>> given we iterate by output port index into an array of out ports, and
>>> have an array of in-ports by index, a third mapping array, same size
>>> as in-ports, determining the associated out port index should suffice.
>>> Mapping array should be optional - if not there, path discovery works
>>> as previously
>>
>> We could simply add a "(sticky)flag" to the
>> "coresight_connection".src_port/dest_port and extend the array to include the
>> sticky_port for src/dest port and use that flag to force the path traversal.
>>
>> Suzuki
>>
>>
>>
>>>
>>> Regards
>>>
>>> Mike
>>>
>>>> Kind regards
>>>> Suzuki
>>>>
>>>>
>>>>>
>>>>> Regards
>>>>>
>>>>> Mike
>>>>>
>>>>>> Rob/Krzysztof,
>>>>>>
>>>>>> Do you have any recommendations for describing the 'hard wired
>>>>>> ports' ?
>>>>>>
>>>>>> e.g:
>>>>>>
>>>>>> component {
>>>>>> input_ports {
>>>>>> component_input_port0: port@0 {
>>>>>> ...
>>>>>> <hard-wired-to*> = &component_output_port0;
>>>>>> };
>>>>>> ...
>>>>>> };
>>>>>>
>>>>>> output_ports {
>>>>>> componentne_output_port0: port@0 {
>>>>>> ...
>>>>>> <hard-wired-to> = &component_input_port0;
>>>>>> };
>>>>>> ...
>>>>>> };
>>>>>>
>>>>>> };
>>>>>>
>>>>>> *Need a better suitable property than "hard-wired-to".
>>>>>>
>>>>>>
>>>>>> Suzuki
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Best,
>>>>>>>
>>>>>>> Tao
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not
>>>>>>>>> sure what's the meaning
>>>>>>>>
>>>>>>>> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is
>>>>>>>> acting like a cache for easing ATB data load, by providing h/w buffering.
>>>>>>>> (In your case, it may not be providing any buffering, it doesn't
>>>>>>>> matter either way, as it is not visible to the driver).
>>>>>>>>
>>>>>>>> Suzuki
>>>>>>>>
>>>>>>>>>
>>>>>>>>> of this. Could you describe it in detail?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Best,
>>>>>>>>>
>>>>>>>>> Tao
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Suzuki
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>>>>>> ---
>>>>>>>>>>> drivers/hwtracing/coresight/coresight-core.c | 81
>>>>>>>>>>> ++++++++++++++++---
>>>>>>>>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>>>>>>>>>>> include/linux/coresight.h | 2 +
>>>>>>>>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>> b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
>>>>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>> @@ -113,15 +113,63 @@ struct coresight_device
>>>>>>>>>>> *coresight_get_percpu_sink(int cpu)
>>>>>>>>>>> }
>>>>>>>>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>>>>>>>>>> +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;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +/**
>>>>>>>>>>> + * 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_label == NULL)
>>>>>>>>>>> + return ret;
>>>>>>>>>>> +
>>>>>>>>>>> + source = coresight_get_source(path);
>>>>>>>>>>> + if (source == NULL)
>>>>>>>>>>> + return ret;
>>>>>>>>>>> +
>>>>>>>>>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>>>>>>>> + conn->source_label))
>>>>>>>>>>> + ret = 0;
>>>>>>>>>>> + else
>>>>>>>>>>> + ret = -1;
>>>>>>>>>>> +
>>>>>>>>>>> + return ret;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> static struct coresight_connection *
>>>>>>>>>>> coresight_find_out_connection(struct coresight_device *src_dev,
>>>>>>>>>>> - struct coresight_device *dest_dev)
>>>>>>>>>>> + struct coresight_device *dest_dev,
>>>>>>>>>>> + struct list_head *path)
>>>>>>>>>>> {
>>>>>>>>>>> int i;
>>>>>>>>>>> struct coresight_connection *conn;
>>>>>>>>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>>>>>>>>> conn = src_dev->pdata->out_conns[i];
>>>>>>>>>>> + if (coresight_source_filter(path, conn))
>>>>>>>>>>> + continue;
>>>>>>>>>>> if (conn->dest_dev == dest_dev)
>>>>>>>>>>> return conn;
>>>>>>>>>>> }
>>>>>>>>>>> @@ -312,7 +360,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;
>>>>>>>>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>> if (!parent || !child)
>>>>>>>>>>> return -EINVAL;
>>>>>>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>>>>>>>>> + outconn = coresight_find_out_connection(csdev, child,
>>>>>>>>>>> + path);
>>>>>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG
>>>>>>>>>>> &&
>>>>>>>>>>> IS_ERR(inconn))
>>>>>>>>>>> @@ -341,7 +390,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;
>>>>>>>>>>> int link_subtype;
>>>>>>>>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>> if (!parent || !child)
>>>>>>>>>>> return;
>>>>>>>>>>> - inconn = coresight_find_out_connection(parent, csdev);
>>>>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>>>>>>>>>> + outconn = coresight_find_out_connection(csdev, child,
>>>>>>>>>>> + path);
>>>>>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>>>>>> if (link_ops(csdev)->disable) { @@ -507,7 +557,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;
>>>>>>>>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
>>>>>>>>>>> *path, enum cs_mode mode,
>>>>>>>>>>> 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;
>>>>>>>>>>> @@ -802,7 +852,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;
>>>>>>>>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>> if (coresight_is_percpu_source(csdev) &&
>>>>>>>>>>> coresight_is_percpu_sink(sink) &&
>>>>>>>>>>> sink == per_cpu(csdev_sink,
>>>>>>>>>>> source_ops(csdev)->cpu_id(csdev))) {
>>>>>>>>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
>>>>>>>>>>> + if (_coresight_build_path(sink, sink, path, source)
>>>>>>>>>>> + == 0) {
>>>>>>>>>>> found = true;
>>>>>>>>>>> goto out;
>>>>>>>>>>> }
>>>>>>>>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>> struct coresight_device *child_dev;
>>>>>>>>>>> child_dev =
>>>>>>>>>>> csdev->pdata->out_conns[i]->dest_dev;
>>>>>>>>>>> + if (csdev->pdata->out_conns[i]->source_label &&
>>>>>>>>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>>>>>>>> + csdev->pdata->out_conns[i]->source_label))
>>>>>>>>>>> + continue;
>>>>>>>>>>> if (child_dev &&
>>>>>>>>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>>>>>>>>>> + _coresight_build_path(child_dev, sink, path,
>>>>>>>>>>> + source)
>>>>>>>>>>> == 0) {
>>>>>>>>>>> found = true;
>>>>>>>>>>> break;
>>>>>>>>>>> }
>>>>>>>>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
>>>>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>>>>>> @@ -205,6 +205,7 @@ static int
>>>>>>>>>>> of_coresight_parse_endpoint(struct
>>>>>>>>>>> device *dev,
>>>>>>>>>>> struct fwnode_handle *rdev_fwnode;
>>>>>>>>>>> struct coresight_connection conn = {};
>>>>>>>>>>> struct coresight_connection *new_conn;
>>>>>>>>>>> + const char *label;
>>>>>>>>>>> do {
>>>>>>>>>>> /* Parse the local port details */ @@ -243,6
>>>>>>>>>>> +244,10 @@ static int of_coresight_parse_endpoint(struct
>>>>>>>>>>> device *dev,
>>>>>>>>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>>>>>>>>>> conn.dest_port = rendpoint.port;
>>>>>>>>>>> + conn.source_label = NULL;
>>>>>>>>>>> + if (!of_property_read_string(ep, "label", &label))
>>>>>>>>>>> + conn.source_label = label;
>>>>>>>>>>> +
>>>>>>>>>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
>>>>>>>>>>> if (IS_ERR_VALUE(new_conn)) {
>>>>>>>>>>> fwnode_handle_put(conn.dest_fwnode);
>>>>>>>>>>> diff --git a/include/linux/coresight.h
>>>>>>>>>>> b/include/linux/coresight.h index e8b6e388218c..a9c06ef9bbb2
>>>>>>>>>>> 100644
>>>>>>>>>>> --- a/include/linux/coresight.h
>>>>>>>>>>> +++ b/include/linux/coresight.h
>>>>>>>>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
>>>>>>>>>>> * struct coresight_connection - representation of a
>>>>>>>>>>> single connection
>>>>>>>>>>> * @src_port: a connection's output port number.
>>>>>>>>>>> * @dest_port: destination's input port number @src_port is
>>>>>>>>>>> connected to.
>>>>>>>>>>> + * @source_label: source component's label.
>>>>>>>>>>> * @dest_fwnode: destination component's fwnode handle.
>>>>>>>>>>> * @dest_dev: a @coresight_device representation of the
>> component
>>>>>>>>>>> connected to @src_port. NULL until the device is
>>>>>>>>>>> created @@ -195,6 +196,7 @@ struct coresight_desc {
>>>>>>>>>>> struct coresight_connection {
>>>>>>>>>>> int src_port;
>>>>>>>>>>> int dest_port;
>>>>>>>>>>> + const char *source_label;
>>>>>>>>>>> struct fwnode_handle *dest_fwnode;
>>>>>>>>>>> struct coresight_device *dest_dev;
>>>>>>>>>>> struct coresight_sysfs_link *link;
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>> _______________________________________________
>> 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]
--
Thanks,
Tingwei
On 18/04/2024 13:01, Tingwei Zhang wrote:
> Hi Mike and Suzuki
>
> On 4/18/2024 5:25 PM, Mike Leach wrote:
>> Hi Suzuki
>>
>>> -----Original Message-----
>>> From: Suzuki K Poulose <[email protected]>
>>> Sent: Thursday, April 18, 2024 10:00 AM
>>> To: Mike Leach <[email protected]>
>>> Cc: Tao Zhang <[email protected]>; Mathieu Poirier
>>> <[email protected]>; Alexander Shishkin
>>> <[email protected]>; Konrad Dybcio
>>> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
>>> Kozlowski <[email protected]>; Jinlong Mao
>>> <[email protected]>; Greg Kroah-Hartman
>>> <[email protected]>; [email protected]; linux-arm-
>>> [email protected]; [email protected];
>>> [email protected]; Tingwei Zhang <[email protected]>;
>>> Yuanfang Zhang <[email protected]>; Trilok Soni
>>> <[email protected]>; Song Chai <[email protected]>;
>>> linux-arm-
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH 2/4] coresight: Add support for multiple output
>>> ports on the
>>> funnel
>>>
>>> Hi Mike
>>>
>>> On 18/04/2024 09:48, Mike Leach wrote:
>>>> Hi Suzuki
>>>>
>>>> On Wed, 17 Apr 2024 at 10:21, Suzuki K Poulose <[email protected]>
>>> wrote:
>>>>>
>>>>> Hi Mike
>>>>>
>>>>> On 16/04/2024 15:19, Mike Leach wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Mon, 15 Apr 2024 at 14:24, Suzuki K Poulose
>>>>>> <[email protected]>
>>> wrote:
>>>>>>>
>>>>>>> On 09/04/2024 14:22, Tao Zhang wrote:
>>>>>>>>
>>>>>>>> On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On 29/03/2024 09:27, Tao Zhang wrote:
>>>>>>>>>>
>>>>>>>>>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
>>>>>>>>>>> On 21/03/2024 08:32, Tao Zhang wrote:
>>>>>>>>>>>> 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 port is selected according to the source in
>>>>>>>>>>>> the trace
>>> path.
>>>>>>>>>>>>
>>>>>>>>>>>> The source of the input port on funnels will be marked in the
>>>>>>>>>>>> device tree.
>>>>>>>>>>>> e.g.
>>>>>>>>>>>> tpdm@xxxxxxx {
>>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>>> };
>>>>>>>>>>>>
>>>>>>>>>>>> funnel_XXX: funnel@xxxxxxx {
>>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>>> out-ports {
>>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>>> port@x {
>>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>>> label = "xxxxxxx.tpdm"; <-- To label the source
>>>>>>>>>>>> }; corresponding to
>>>>>>>>>>>> the output
>>>>>>>>>>>> ... ... ... ... connection
>>>>>>>>>>>> "port@x". And this
>>>>>>>>>>>> }; is a hardware static
>>>>>>>>>>>> connections.
>>>>>>>>>>>> ... ... ... ... Here needs to refer
>>>>>>>>>>>> to hardware
>>>>>>>>>>>> }; design.
>>>>>>>>>>>>
>>>>>>>>>>>> Then driver will parse the source label marked in the device
>>>>>>>>>>>> tree, and save it to the coresight path. When the function
>>>>>>>>>>>> needs to know the source label, it could obtain it from
>>>>>>>>>>>> coresight path
>>> parameter.
>>>>>>>>>>>> Finally,
>>>>>>>>>>>> the output port knows which source it corresponds to, and it
>>>>>>>>>>>> also knows which input port it corresponds to.
>>>>>>>>>>>
>>>>>>>>>>> Why do we need labels ? We have connection information for all
>>>>>>>>>>> devices (both in and out), so, why do we need this label to
>>>>>>>>>>> find a device
>>> ?
>>>>>>>>>>
>>>>>>>>>> Because our funnel's design has multi-output ports, the data
>>>>>>>>>> stream will not
>>>>>>>>>>
>>>>>>>>>> know which output port should pass in building the data trace
>>>>>>>>>> path.
>>>>>>>>>> This source
>>>>>>>>>>
>>>>>>>>>> label can make the data stream find the right output port to go.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> And also, I thought TPDM is a source device, why does a funnel
>>>>>>>>>>> output port link to a source ?
>>>>>>>>>>
>>>>>>>>>> No, this label doesn't mean this funnel output port link to a
>>>>>>>>>> source, it just let
>>>>>>>>>>
>>>>>>>>>> the output port know its data source.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Are these funnels programmable ? Or, are they static ? If they
>>>>>>>>>>> are static, do these need to be described in the DT ? If they
>>>>>>>>>>> are simply acting as a "LINK" (or HWFIFO ?)
>>>>>>>>>>
>>>>>>>>>> These funnels are static, and we will add the "label" to the DT
>>>>>>>>>> to describe the
>>>>>>>>>>
>>>>>>>>>> multi-output ports for these funnels.
>>>>>>>>>
>>>>>>>>> I think there is still a bit of confusion. By "Dynamic" I mean,
>>>>>>>>> the "dynamic funnel" (explicit port enablement via MMIO) vs
>>>>>>>>> "static funnel" (no programming, always ON).
>>>>>>>>>
>>>>>>>>> So, coming to your example, do we need to "explicitly" enable
>>>>>>>>> trace flow for an "input" and/or an "output" port in your
>>>>>>>>> "funnel" ?
>>>>>>>>
>>>>>>>> Sorry for my misunderstanding in the previous mails. Our funnels
>>>>>>>> are programmable just like the common dynamic funnels.
>>>>>>>>
>>>>>>>> In our solution, we just make funnels have multiple output ports
>>>>>>>> connected to different devices or ports. When we use it, we still
>>>>>>>>
>>>>>>>> enable the input port through programming. Our solution is to know
>>>>>>>> which input port the expected data comes from based on the
>>>>>>>>
>>>>>>>> source label corresponding to the output port. This way we can
>>>>>>>> build the expected trace path. In other respects, it is used the
>>>>>>>> same
>>>>>>>>
>>>>>>>> as common dynamic funnels.
>>>>>>>
>>>>>>>
>>>>>>> Ok. So, to summarise :
>>>>>>>
>>>>>>> 1. This is not a standard Funnel, but a trace link with
>>>>>>> multiple-input
>>>>>>> and multiple-output, with inputs hardwired to an outline at
>>>>>>> integration.
>>>>>>> 2. The programming model is same as that of a "standard funnel".
>>>>>>>
>>>>>>> Now, we do have enough information in the coresight_connections to
>>>>>>> traverse input/output ports. But we need additional logic to
>>>>>>> "hardwire" the ports to each other and necessary logic to handle
>>>>>>> the
>>>>>>>
>>>>>>> There are two options here :
>>>>>>>
>>>>>>> 1. Treat this as a new component and have its own driver, with
>>>>>>> additional logic to handle the input/output wiring.
>>>>>>>
>>>>>>> 2. Drive it using the funnel driver, with a a new compatible and
>>>>>>> add additional logic to handle the input/output wiring.
>>>>>>>
>>>>>>> My inclination is towards (2), we need to see how this works out.
>>>>>>>
>>>>>>> We need to irrespective of the options, we need special handling
>>>>>>> for hardwired ports in 1) building path 2) walking back the path
>>>>>>> (in TPDA driver)
>>>>>>>
>>>>>>> We also need some "DT" information to bind a given input port to an
>>>>>>> output port. We must not use "any device" labels to hack this up,
>>>>>>> like the approach in this series.
>>>>>>>
>>>>>>
>>>>>> Given that the internal connections are static for the given device,
>>>>>> then the compatible will imply these connections in just the same
>>>>>> way as the arm,coresight-funnel implies that all inputs are
>>>>>> connected to the single output.
>>>>>
>>>>> I am sorry, I couldn't follow the last part. We have two or more
>>>>> output ports and we need a way to identify, which input port is
>>>>> hardwired to
>>>>> output-port0 and output-port1. Given we need special handling for
>>>>> these anyway, I would like to avoid hard coding the input-output
>>>>> connection.
>>>>> i.e., we do not want to assume that input-0 is always => output-0.
>>>>>
>>>>
>>>> If we regard the current component as having compatible
>>>> "qcom,coresight-compound-funnel-v1", then this identifies the
>>>> relationship between the in-ports and out-ports.
>>>> So the new driver / extension to the funnel driver that handles this
>>>> compatible with know the static mapping between input and output so
>>>> program it.
>>>
>>> Ok, but like I said, having one compatible may not be enough to know
>>> the "static"
>>> mapping for all possible device instances on different SoCs.
>>>
>>
>> The compatible name would have to change if the mapping changed.
>> Using the compatible is simpler, but less flexible
>>
>>>>
>>>> If however you want a more generic approach to handle future different
>>>> versions of the component, then of course a method in DT of mapping
>>>> in-ports to out-ports is useful.
>>>>
>>>> If did wonder if something along the lines of:-
>>>>
>>>> compound-funnel@0x1234000 {
>>>> compatible = "compound-funnel"
>>>> regs = < 0x1234000 0x1000>
>>>> sub-funnel@0 {
>>>> in-ports {
>>>> [some port definitions]
>>>> }
>>>> out-ports {
>>>> [some port definitions]
>>>> }
>>>> }
>>>> sub-funnel@1 {
>>>> in-ports {
>>>> [some port definitions]
>>>> }
>>>> out-ports {
>>>> [some port definitions]
>>>> }
>>>> }
>>>> }
>>>
>>> That would work, with "two" different coresight devices for each
>>> "embedded
>>> funnel". And that also confuses the user with the topology.
>>
>> I wasn't suggesting two different coresight devices, but finding a way
>> to iterate the sub-nodes to create a single device with the inputs
>> mapped to outputs. Which may or may not be easily do-able.
>>
>> As to topology - no more confusing than a "funnel" with multiple
>> outputs, or phandle links between inputs and outputs. It does visually
>> represent what the device really is - multiple effective funnels
>> controlled by a single set of registers.
>>
>
> Let me provide the hardware topology here to facilitate the discussion.
>
> |----------| |---------| |----------| |---------|
> | TPDM 0 | | Source0 | | Source 1 | | TPDM 1 |
> |----------| |---------| |----------| |---------|
> | | | |
> | | | |
> | --------- | | |
> | | | |
> | | | |
> | | | |
> \-------------/ ---------------------- |
> \ Funnel 0 / | |
> ----------- | ---------------------------------
> | | |
> | | |
> \ -------------/
> \ Funnel 1 /
> -----------/
> | |---------------------
> | |------------ |
> | |TPDM0 |TPDM1
> | \ ----------------/
> | \ TPDA 0 /
> | --------------/
> | |
> | |
> |Source0/1 |
> \-------------------------------/
> \ Funnel 2 /
> ---------------------------
>
> To describe this topology in device tree, we need to indicate input
> port0 of FUNNEL0 is static link to output port0 of FUNNEL0 which links
> to input port0 of TPDA0. When code builds the path, it can get the
This is making things even worse. Hold on there. Please could you
confirm the topology again via the "Funnel 1". You seemed to have
skipped it in the path description.
How many outputs does the Funnel 1 have ? 2 or 3 ? How does the Funnel 1
know to split the "data" from Funnel1:input0 (which could be either
Source0 or TPDM0) ?
> static link information from topology and select correct path. As Suzuki
> has suggested, we can describe the topology like below.
No, I suggested hardwiring the output-input of "the same component". Not
of the two different components. I guess we need to sort this out
offline, (happy to setup a call to understand and clear things better).
>
> funnel0 {
> ...
> in-ports {
> port@0 {
> funnel0_in0: endpoint {
> remote-endpoint = <&tpdm0_output>;
> <hard-wired-to*>=
> <&funnel1_out0>;
No, I meant. Not skipping the components.
<&funnel0_out0>;
This leaves the question of how Funnel1 does the separation, unless
it is a replicator and does the filtering based on the TraceID.
Suzuki
> }
> }
> }
> }
>
> funnel1 {
> ...
> out-ports {
> port@0 {
> funnel1_out0: endpoint {
> remote-endpoint= <&tpdm0_in0>;
> <hard-wired-to*>=
> <&funnel0_in0>;
> }
> }
> }
> }
>
>
>> Mike
>>
>>>
>>>>
>>>>
>>>> might be made to work? not sure about the implications of having more
>>>> that one set of in-ports and out-ports in a component in the device
>>>> tree & would need specific handling in the driver to iterate
>>>> sub-funnels.
>>>>
>>>>
>>>>>
>>>>>>
>>>>>> Irrespective of if a new driver is used, or an extension to the
>>>>>> current funnel driver to handle a new compatible - the mapping
>>>>>> between input and output ports are created based on the compatible..
>>>>>>
>>>>>> As we are building a path from source to sink, what is then needed
>>>>>> is a method in the generic path building code, to recognise these
>>>>>> amppings and filter the output ports that are searched based on the
>>>>>> input port in use.
>>>>>
>>>>> Agreed. We could mark this as a property of the
>>>>> port/coresight_connection.
>>>>>
>>>>>>
>>>>>> On standard components, where the mapping is not present, then the
>>>>>> code will continue as it does now, for these compound funnels, the
>>>>>> mappings will be present and the output filtering will occur.
>>>>>
>>>>> Agreed
>>>>>
>>>>>> This removes the need for the labels / extra connection attributes
>>>>>> on devices other than the funnel, and also removes the need to
>>>>>> specify the internal connections as part of the device tree.
>>>>>
>>>>> I am still not clear how we map the input-output ports. Rest is what
>>>>> exactly I had in mind. So, once we sort out the port mapping we could
>>>>> proceed to the prototyping.
>>>>>
>>>>
>>>> given we iterate by output port index into an array of out ports, and
>>>> have an array of in-ports by index, a third mapping array, same size
>>>> as in-ports, determining the associated out port index should suffice.
>>>> Mapping array should be optional - if not there, path discovery works
>>>> as previously
>>>
>>> We could simply add a "(sticky)flag" to the
>>> "coresight_connection".src_port/dest_port and extend the array to
>>> include the
>>> sticky_port for src/dest port and use that flag to force the path
>>> traversal.
>>>
>>> Suzuki
>>>
>>>
>>>
>>>>
>>>> Regards
>>>>
>>>> Mike
>>>>
>>>>> Kind regards
>>>>> Suzuki
>>>>>
>>>>>
>>>>>>
>>>>>> Regards
>>>>>>
>>>>>> Mike
>>>>>>
>>>>>>> Rob/Krzysztof,
>>>>>>>
>>>>>>> Do you have any recommendations for describing the 'hard wired
>>>>>>> ports' ?
>>>>>>>
>>>>>>> e.g:
>>>>>>>
>>>>>>> component {
>>>>>>> input_ports {
>>>>>>> component_input_port0: port@0 {
>>>>>>> ...
>>>>>>> <hard-wired-to*> = &component_output_port0;
>>>>>>> };
>>>>>>> ...
>>>>>>> };
>>>>>>>
>>>>>>> output_ports {
>>>>>>> componentne_output_port0: port@0 {
>>>>>>> ...
>>>>>>> <hard-wired-to> = &component_input_port0;
>>>>>>> };
>>>>>>> ...
>>>>>>> };
>>>>>>>
>>>>>>> };
>>>>>>>
>>>>>>> *Need a better suitable property than "hard-wired-to".
>>>>>>>
>>>>>>>
>>>>>>> Suzuki
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Best,
>>>>>>>>
>>>>>>>> Tao
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not
>>>>>>>>>> sure what's the meaning
>>>>>>>>>
>>>>>>>>> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is
>>>>>>>>> acting like a cache for easing ATB data load, by providing h/w
>>>>>>>>> buffering.
>>>>>>>>> (In your case, it may not be providing any buffering, it doesn't
>>>>>>>>> matter either way, as it is not visible to the driver).
>>>>>>>>>
>>>>>>>>> Suzuki
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> of this. Could you describe it in detail?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Best,
>>>>>>>>>>
>>>>>>>>>> Tao
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Suzuki
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>>>>>>> ---
>>>>>>>>>>>> drivers/hwtracing/coresight/coresight-core.c | 81
>>>>>>>>>>>> ++++++++++++++++---
>>>>>>>>>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>>>>>>>>>>>> include/linux/coresight.h | 2 +
>>>>>>>>>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>>> b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
>>>>>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>>> @@ -113,15 +113,63 @@ struct coresight_device
>>>>>>>>>>>> *coresight_get_percpu_sink(int cpu)
>>>>>>>>>>>> }
>>>>>>>>>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>>>>>>>>>>> +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;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>> +/**
>>>>>>>>>>>> + * 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_label == NULL)
>>>>>>>>>>>> + return ret;
>>>>>>>>>>>> +
>>>>>>>>>>>> + source = coresight_get_source(path);
>>>>>>>>>>>> + if (source == NULL)
>>>>>>>>>>>> + return ret;
>>>>>>>>>>>> +
>>>>>>>>>>>> + if (strstr(kobject_get_path(&source->dev.kobj,
>>>>>>>>>>>> GFP_KERNEL),
>>>>>>>>>>>> + conn->source_label))
>>>>>>>>>>>> + ret = 0;
>>>>>>>>>>>> + else
>>>>>>>>>>>> + ret = -1;
>>>>>>>>>>>> +
>>>>>>>>>>>> + return ret;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>> static struct coresight_connection *
>>>>>>>>>>>> coresight_find_out_connection(struct coresight_device
>>>>>>>>>>>> *src_dev,
>>>>>>>>>>>> - struct coresight_device *dest_dev)
>>>>>>>>>>>> + struct coresight_device *dest_dev,
>>>>>>>>>>>> + struct list_head *path)
>>>>>>>>>>>> {
>>>>>>>>>>>> int i;
>>>>>>>>>>>> struct coresight_connection *conn;
>>>>>>>>>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>>>>>>>>>> conn = src_dev->pdata->out_conns[i];
>>>>>>>>>>>> + if (coresight_source_filter(path, conn))
>>>>>>>>>>>> + continue;
>>>>>>>>>>>> if (conn->dest_dev == dest_dev)
>>>>>>>>>>>> return conn;
>>>>>>>>>>>> }
>>>>>>>>>>>> @@ -312,7 +360,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;
>>>>>>>>>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>>> if (!parent || !child)
>>>>>>>>>>>> return -EINVAL;
>>>>>>>>>>>> - inconn = coresight_find_out_connection(parent,
>>>>>>>>>>>> csdev);
>>>>>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev,
>>>>>>>>>>>> path);
>>>>>>>>>>>> + outconn = coresight_find_out_connection(csdev, child,
>>>>>>>>>>>> + path);
>>>>>>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>>>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG
>>>>>>>>>>>> &&
>>>>>>>>>>>> IS_ERR(inconn))
>>>>>>>>>>>> @@ -341,7 +390,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;
>>>>>>>>>>>> int link_subtype;
>>>>>>>>>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>>> if (!parent || !child)
>>>>>>>>>>>> return;
>>>>>>>>>>>> - inconn = coresight_find_out_connection(parent,
>>>>>>>>>>>> csdev);
>>>>>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev,
>>>>>>>>>>>> path);
>>>>>>>>>>>> + outconn = coresight_find_out_connection(csdev, child,
>>>>>>>>>>>> + path);
>>>>>>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>>>>>>> if (link_ops(csdev)->disable) { @@ -507,7 +557,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;
>>>>>>>>>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
>>>>>>>>>>>> *path, enum cs_mode mode,
>>>>>>>>>>>> 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;
>>>>>>>>>>>> @@ -802,7 +852,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;
>>>>>>>>>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>>> if (coresight_is_percpu_source(csdev) &&
>>>>>>>>>>>> coresight_is_percpu_sink(sink) &&
>>>>>>>>>>>> sink == per_cpu(csdev_sink,
>>>>>>>>>>>> source_ops(csdev)->cpu_id(csdev))) {
>>>>>>>>>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
>>>>>>>>>>>> + if (_coresight_build_path(sink, sink, path, source)
>>>>>>>>>>>> + == 0) {
>>>>>>>>>>>> found = true;
>>>>>>>>>>>> goto out;
>>>>>>>>>>>> }
>>>>>>>>>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>>> struct coresight_device *child_dev;
>>>>>>>>>>>> child_dev =
>>>>>>>>>>>> csdev->pdata->out_conns[i]->dest_dev;
>>>>>>>>>>>> + if (csdev->pdata->out_conns[i]->source_label &&
>>>>>>>>>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>>>>>>>>> + csdev->pdata->out_conns[i]->source_label))
>>>>>>>>>>>> + continue;
>>>>>>>>>>>> if (child_dev &&
>>>>>>>>>>>> - _coresight_build_path(child_dev, sink, path) ==
>>>>>>>>>>>> 0) {
>>>>>>>>>>>> + _coresight_build_path(child_dev, sink, path,
>>>>>>>>>>>> + source)
>>>>>>>>>>>> == 0) {
>>>>>>>>>>>> found = true;
>>>>>>>>>>>> break;
>>>>>>>>>>>> }
>>>>>>>>>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
>>>>>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>>>>>>> @@ -205,6 +205,7 @@ static int
>>>>>>>>>>>> of_coresight_parse_endpoint(struct
>>>>>>>>>>>> device *dev,
>>>>>>>>>>>> struct fwnode_handle *rdev_fwnode;
>>>>>>>>>>>> struct coresight_connection conn = {};
>>>>>>>>>>>> struct coresight_connection *new_conn;
>>>>>>>>>>>> + const char *label;
>>>>>>>>>>>> do {
>>>>>>>>>>>> /* Parse the local port details */ @@ -243,6
>>>>>>>>>>>> +244,10 @@ static int of_coresight_parse_endpoint(struct
>>>>>>>>>>>> device *dev,
>>>>>>>>>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>>>>>>>>>>> conn.dest_port = rendpoint.port;
>>>>>>>>>>>> + conn.source_label = NULL;
>>>>>>>>>>>> + if (!of_property_read_string(ep, "label", &label))
>>>>>>>>>>>> + conn.source_label = label;
>>>>>>>>>>>> +
>>>>>>>>>>>> new_conn = coresight_add_out_conn(dev, pdata,
>>>>>>>>>>>> &conn);
>>>>>>>>>>>> if (IS_ERR_VALUE(new_conn)) {
>>>>>>>>>>>> fwnode_handle_put(conn.dest_fwnode);
>>>>>>>>>>>> diff --git a/include/linux/coresight.h
>>>>>>>>>>>> b/include/linux/coresight.h index e8b6e388218c..a9c06ef9bbb2
>>>>>>>>>>>> 100644
>>>>>>>>>>>> --- a/include/linux/coresight.h
>>>>>>>>>>>> +++ b/include/linux/coresight.h
>>>>>>>>>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
>>>>>>>>>>>> * struct coresight_connection - representation of a
>>>>>>>>>>>> single connection
>>>>>>>>>>>> * @src_port: a connection's output port number.
>>>>>>>>>>>> * @dest_port: destination's input port number
>>>>>>>>>>>> @src_port is
>>>>>>>>>>>> connected to.
>>>>>>>>>>>> + * @source_label: source component's label.
>>>>>>>>>>>> * @dest_fwnode: destination component's fwnode handle.
>>>>>>>>>>>> * @dest_dev: a @coresight_device representation of the
>>> component
>>>>>>>>>>>> connected to @src_port. NULL until the device is
>>>>>>>>>>>> created @@ -195,6 +196,7 @@ struct coresight_desc {
>>>>>>>>>>>> struct coresight_connection {
>>>>>>>>>>>> int src_port;
>>>>>>>>>>>> int dest_port;
>>>>>>>>>>>> + const char *source_label;
>>>>>>>>>>>> struct fwnode_handle *dest_fwnode;
>>>>>>>>>>>> struct coresight_device *dest_dev;
>>>>>>>>>>>> struct coresight_sysfs_link *link;
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>> _______________________________________________
>>> 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]
>
On 4/18/2024 8:38 PM, Suzuki K Poulose wrote:
> On 18/04/2024 13:01, Tingwei Zhang wrote:
>> Hi Mike and Suzuki
>>
>> On 4/18/2024 5:25 PM, Mike Leach wrote:
>>> Hi Suzuki
>>>
>>>> -----Original Message-----
>>>> From: Suzuki K Poulose <[email protected]>
>>>> Sent: Thursday, April 18, 2024 10:00 AM
>>>> To: Mike Leach <[email protected]>
>>>> Cc: Tao Zhang <[email protected]>; Mathieu Poirier
>>>> <[email protected]>; Alexander Shishkin
>>>> <[email protected]>; Konrad Dybcio
>>>> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
>>>> Kozlowski <[email protected]>; Jinlong Mao
>>>> <[email protected]>; Greg Kroah-Hartman
>>>> <[email protected]>; [email protected]; linux-arm-
>>>> [email protected]; [email protected];
>>>> [email protected]; Tingwei Zhang <[email protected]>;
>>>> Yuanfang Zhang <[email protected]>; Trilok Soni
>>>> <[email protected]>; Song Chai <[email protected]>;
>>>> linux-arm-
>>>> [email protected]; [email protected]
>>>> Subject: Re: [PATCH 2/4] coresight: Add support for multiple output
>>>> ports on the
>>>> funnel
>>>>
>>>> Hi Mike
>>>>
>>>> On 18/04/2024 09:48, Mike Leach wrote:
>>>>> Hi Suzuki
>>>>>
>>>>> On Wed, 17 Apr 2024 at 10:21, Suzuki K Poulose
>>>>> <[email protected]>
>>>> wrote:
>>>>>>
>>>>>> Hi Mike
>>>>>>
>>>>>> On 16/04/2024 15:19, Mike Leach wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Mon, 15 Apr 2024 at 14:24, Suzuki K Poulose
>>>>>>> <[email protected]>
>>>> wrote:
>>>>>>>>
>>>>>>>> On 09/04/2024 14:22, Tao Zhang wrote:
>>>>>>>>>
>>>>>>>>> On 4/9/2024 3:13 PM, Suzuki K Poulose wrote:
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> On 29/03/2024 09:27, Tao Zhang wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
>>>>>>>>>>>> On 21/03/2024 08:32, Tao Zhang wrote:
>>>>>>>>>>>>> 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 port is selected according to the source
>>>>>>>>>>>>> in the trace
>>>> path.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The source of the input port on funnels will be marked in the
>>>>>>>>>>>>> device tree.
>>>>>>>>>>>>> e.g.
>>>>>>>>>>>>> tpdm@xxxxxxx {
>>>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>>>> };
>>>>>>>>>>>>>
>>>>>>>>>>>>> funnel_XXX: funnel@xxxxxxx {
>>>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>>>> out-ports {
>>>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>>>> port@x {
>>>>>>>>>>>>> ... ... ... ...
>>>>>>>>>>>>> label = "xxxxxxx.tpdm"; <-- To label the
>>>>>>>>>>>>> source
>>>>>>>>>>>>> }; corresponding to
>>>>>>>>>>>>> the output
>>>>>>>>>>>>> ... ... ... ... connection
>>>>>>>>>>>>> "port@x". And this
>>>>>>>>>>>>> }; is a hardware static
>>>>>>>>>>>>> connections.
>>>>>>>>>>>>> ... ... ... ... Here needs to
>>>>>>>>>>>>> refer to hardware
>>>>>>>>>>>>> }; design.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Then driver will parse the source label marked in the device
>>>>>>>>>>>>> tree, and save it to the coresight path. When the function
>>>>>>>>>>>>> needs to know the source label, it could obtain it from
>>>>>>>>>>>>> coresight path
>>>> parameter.
>>>>>>>>>>>>> Finally,
>>>>>>>>>>>>> the output port knows which source it corresponds to, and it
>>>>>>>>>>>>> also knows which input port it corresponds to.
>>>>>>>>>>>>
>>>>>>>>>>>> Why do we need labels ? We have connection information for all
>>>>>>>>>>>> devices (both in and out), so, why do we need this label to
>>>>>>>>>>>> find a device
>>>> ?
>>>>>>>>>>>
>>>>>>>>>>> Because our funnel's design has multi-output ports, the data
>>>>>>>>>>> stream will not
>>>>>>>>>>>
>>>>>>>>>>> know which output port should pass in building the data trace
>>>>>>>>>>> path.
>>>>>>>>>>> This source
>>>>>>>>>>>
>>>>>>>>>>> label can make the data stream find the right output port to go.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> And also, I thought TPDM is a source device, why does a funnel
>>>>>>>>>>>> output port link to a source ?
>>>>>>>>>>>
>>>>>>>>>>> No, this label doesn't mean this funnel output port link to a
>>>>>>>>>>> source, it just let
>>>>>>>>>>>
>>>>>>>>>>> the output port know its data source.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Are these funnels programmable ? Or, are they static ? If they
>>>>>>>>>>>> are static, do these need to be described in the DT ? If they
>>>>>>>>>>>> are simply acting as a "LINK" (or HWFIFO ?)
>>>>>>>>>>>
>>>>>>>>>>> These funnels are static, and we will add the "label" to the DT
>>>>>>>>>>> to describe the
>>>>>>>>>>>
>>>>>>>>>>> multi-output ports for these funnels.
>>>>>>>>>>
>>>>>>>>>> I think there is still a bit of confusion. By "Dynamic" I mean,
>>>>>>>>>> the "dynamic funnel" (explicit port enablement via MMIO) vs
>>>>>>>>>> "static funnel" (no programming, always ON).
>>>>>>>>>>
>>>>>>>>>> So, coming to your example, do we need to "explicitly" enable
>>>>>>>>>> trace flow for an "input" and/or an "output" port in your
>>>>>>>>>> "funnel" ?
>>>>>>>>>
>>>>>>>>> Sorry for my misunderstanding in the previous mails. Our funnels
>>>>>>>>> are programmable just like the common dynamic funnels.
>>>>>>>>>
>>>>>>>>> In our solution, we just make funnels have multiple output ports
>>>>>>>>> connected to different devices or ports. When we use it, we still
>>>>>>>>>
>>>>>>>>> enable the input port through programming. Our solution is to know
>>>>>>>>> which input port the expected data comes from based on the
>>>>>>>>>
>>>>>>>>> source label corresponding to the output port. This way we can
>>>>>>>>> build the expected trace path. In other respects, it is used the
>>>>>>>>> same
>>>>>>>>>
>>>>>>>>> as common dynamic funnels.
>>>>>>>>
>>>>>>>>
>>>>>>>> Ok. So, to summarise :
>>>>>>>>
>>>>>>>> 1. This is not a standard Funnel, but a trace link with
>>>>>>>> multiple-input
>>>>>>>> and multiple-output, with inputs hardwired to an outline at
>>>>>>>> integration.
>>>>>>>> 2. The programming model is same as that of a "standard funnel".
>>>>>>>>
>>>>>>>> Now, we do have enough information in the coresight_connections to
>>>>>>>> traverse input/output ports. But we need additional logic to
>>>>>>>> "hardwire" the ports to each other and necessary logic to handle
>>>>>>>> the
>>>>>>>>
>>>>>>>> There are two options here :
>>>>>>>>
>>>>>>>> 1. Treat this as a new component and have its own driver, with
>>>>>>>> additional logic to handle the input/output wiring.
>>>>>>>>
>>>>>>>> 2. Drive it using the funnel driver, with a a new compatible and
>>>>>>>> add additional logic to handle the input/output wiring.
>>>>>>>>
>>>>>>>> My inclination is towards (2), we need to see how this works out.
>>>>>>>>
>>>>>>>> We need to irrespective of the options, we need special handling
>>>>>>>> for hardwired ports in 1) building path 2) walking back the path
>>>>>>>> (in TPDA driver)
>>>>>>>>
>>>>>>>> We also need some "DT" information to bind a given input port to an
>>>>>>>> output port. We must not use "any device" labels to hack this up,
>>>>>>>> like the approach in this series.
>>>>>>>>
>>>>>>>
>>>>>>> Given that the internal connections are static for the given device,
>>>>>>> then the compatible will imply these connections in just the same
>>>>>>> way as the arm,coresight-funnel implies that all inputs are
>>>>>>> connected to the single output.
>>>>>>
>>>>>> I am sorry, I couldn't follow the last part. We have two or more
>>>>>> output ports and we need a way to identify, which input port is
>>>>>> hardwired to
>>>>>> output-port0 and output-port1. Given we need special handling for
>>>>>> these anyway, I would like to avoid hard coding the input-output
>>>>>> connection.
>>>>>> i.e., we do not want to assume that input-0 is always => output-0.
>>>>>>
>>>>>
>>>>> If we regard the current component as having compatible
>>>>> "qcom,coresight-compound-funnel-v1", then this identifies the
>>>>> relationship between the in-ports and out-ports.
>>>>> So the new driver / extension to the funnel driver that handles this
>>>>> compatible with know the static mapping between input and output so
>>>>> program it.
>>>>
>>>> Ok, but like I said, having one compatible may not be enough to know
>>>> the "static"
>>>> mapping for all possible device instances on different SoCs.
>>>>
>>>
>>> The compatible name would have to change if the mapping changed.
>>> Using the compatible is simpler, but less flexible
>>>
>>>>>
>>>>> If however you want a more generic approach to handle future different
>>>>> versions of the component, then of course a method in DT of mapping
>>>>> in-ports to out-ports is useful.
>>>>>
>>>>> If did wonder if something along the lines of:-
>>>>>
>>>>> compound-funnel@0x1234000 {
>>>>> compatible = "compound-funnel"
>>>>> regs = < 0x1234000 0x1000>
>>>>> sub-funnel@0 {
>>>>> in-ports {
>>>>> [some port definitions]
>>>>> }
>>>>> out-ports {
>>>>> [some port definitions]
>>>>> }
>>>>> }
>>>>> sub-funnel@1 {
>>>>> in-ports {
>>>>> [some port definitions]
>>>>> }
>>>>> out-ports {
>>>>> [some port definitions]
>>>>> }
>>>>> }
>>>>> }
>>>>
>>>> That would work, with "two" different coresight devices for each
>>>> "embedded
>>>> funnel". And that also confuses the user with the topology.
>>>
>>> I wasn't suggesting two different coresight devices, but finding a
>>> way to iterate the sub-nodes to create a single device with the
>>> inputs mapped to outputs. Which may or may not be easily do-able.
>>>
>>> As to topology - no more confusing than a "funnel" with multiple
>>> outputs, or phandle links between inputs and outputs. It does
>>> visually represent what the device really is - multiple effective
>>> funnels controlled by a single set of registers.
>>>
>>
>> Let me provide the hardware topology here to facilitate the discussion.
>>
>> |----------| |---------| |----------| |---------|
>> | TPDM 0 | | Source0 | | Source 1 | | TPDM 1 |
>> |----------| |---------| |----------| |---------|
>> | | | |
>> | | | |
>> | --------- | | |
>> | | | |
>> | | | |
>> | | | |
>> \-------------/ ---------------------- |
>> \ Funnel 0 / | |
>> ----------- | ---------------------------------
>> | | |
>> | | |
>> \ -------------/
>> \ Funnel 1 /
>> -----------/
>> | |---------------------
>> | |------------ |
>> | |TPDM0 |TPDM1
>> | \ ----------------/
>> | \ TPDA 0 /
>> | --------------/
>> | |
>> | |
>> |Source0/1 |
>> \-------------------------------/
>> \ Funnel 2 /
>> ---------------------------
>>
>> To describe this topology in device tree, we need to indicate input
>> port0 of FUNNEL0 is static link to output port0 of FUNNEL0 which links
>> to input port0 of TPDA0. When code builds the path, it can get the
>
> This is making things even worse. Hold on there. Please could you
> confirm the topology again via the "Funnel 1". You seemed to have
> skipped it in the path description.
>
Funnel0 has two inputs and one output. Funnel1 has three inputs and
three outputs.
>
> How many outputs does the Funnel 1 have ? 2 or 3 ? How does the Funnel 1
> know to split the "data" from Funnel1:input0 (which could be either
> Source0 or TPDM0) ?
>
>
>> static link information from topology and select correct path. As
>> Suzuki has suggested, we can describe the topology like below.
>
> No, I suggested hardwiring the output-input of "the same component". Not
> of the two different components. I guess we need to sort this out
> offline, (happy to setup a call to understand and clear things better).
I agree we could have a call to I can clarify things better. Let me get
some data from hardware team on how funnel1 routes data from different
source. Do you have preference time to have that call?
>
>
>>
>> funnel0 {
>> ...
>> in-ports {
>> port@0 {
>> funnel0_in0: endpoint {
>> remote-endpoint = <&tpdm0_output>;
>> <hard-wired-to*>=
>> <&funnel1_out0>;
>
> No, I meant. Not skipping the components.
>
> <&funnel0_out0>;
>
> This leaves the question of how Funnel1 does the separation, unless
> it is a replicator and does the filtering based on the TraceID.
>
>
> Suzuki
>
>
>> }
>> }
>> }
>> }
>>
>> funnel1 {
>> ...
>> out-ports {
>> port@0 {
>> funnel1_out0: endpoint {
>> remote-endpoint= <&tpdm0_in0>;
>> <hard-wired-to*>=
>> <&funnel0_in0>;
>> }
>> }
>> }
>> }
>>
>>
>>> Mike
>>>
>>>>
>>>>>
>>>>>
>>>>> might be made to work? not sure about the implications of having more
>>>>> that one set of in-ports and out-ports in a component in the device
>>>>> tree & would need specific handling in the driver to iterate
>>>>> sub-funnels.
>>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Irrespective of if a new driver is used, or an extension to the
>>>>>>> current funnel driver to handle a new compatible - the mapping
>>>>>>> between input and output ports are created based on the compatible..
>>>>>>>
>>>>>>> As we are building a path from source to sink, what is then needed
>>>>>>> is a method in the generic path building code, to recognise these
>>>>>>> amppings and filter the output ports that are searched based on the
>>>>>>> input port in use.
>>>>>>
>>>>>> Agreed. We could mark this as a property of the
>>>>>> port/coresight_connection.
>>>>>>
>>>>>>>
>>>>>>> On standard components, where the mapping is not present, then the
>>>>>>> code will continue as it does now, for these compound funnels, the
>>>>>>> mappings will be present and the output filtering will occur.
>>>>>>
>>>>>> Agreed
>>>>>>
>>>>>>> This removes the need for the labels / extra connection attributes
>>>>>>> on devices other than the funnel, and also removes the need to
>>>>>>> specify the internal connections as part of the device tree.
>>>>>>
>>>>>> I am still not clear how we map the input-output ports. Rest is what
>>>>>> exactly I had in mind. So, once we sort out the port mapping we could
>>>>>> proceed to the prototyping.
>>>>>>
>>>>>
>>>>> given we iterate by output port index into an array of out ports, and
>>>>> have an array of in-ports by index, a third mapping array, same size
>>>>> as in-ports, determining the associated out port index should suffice.
>>>>> Mapping array should be optional - if not there, path discovery works
>>>>> as previously
>>>>
>>>> We could simply add a "(sticky)flag" to the
>>>> "coresight_connection".src_port/dest_port and extend the array to
>>>> include the
>>>> sticky_port for src/dest port and use that flag to force the path
>>>> traversal.
>>>>
>>>> Suzuki
>>>>
>>>>
>>>>
>>>>>
>>>>> Regards
>>>>>
>>>>> Mike
>>>>>
>>>>>> Kind regards
>>>>>> Suzuki
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Regards
>>>>>>>
>>>>>>> Mike
>>>>>>>
>>>>>>>> Rob/Krzysztof,
>>>>>>>>
>>>>>>>> Do you have any recommendations for describing the 'hard wired
>>>>>>>> ports' ?
>>>>>>>>
>>>>>>>> e.g:
>>>>>>>>
>>>>>>>> component {
>>>>>>>> input_ports {
>>>>>>>> component_input_port0: port@0 {
>>>>>>>> ...
>>>>>>>> <hard-wired-to*> = &component_output_port0;
>>>>>>>> };
>>>>>>>> ...
>>>>>>>> };
>>>>>>>>
>>>>>>>> output_ports {
>>>>>>>> componentne_output_port0: port@0 {
>>>>>>>> ...
>>>>>>>> <hard-wired-to> = &component_input_port0;
>>>>>>>> };
>>>>>>>> ...
>>>>>>>> };
>>>>>>>>
>>>>>>>> };
>>>>>>>>
>>>>>>>> *Need a better suitable property than "hard-wired-to".
>>>>>>>>
>>>>>>>>
>>>>>>>> Suzuki
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Best,
>>>>>>>>>
>>>>>>>>> Tao
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not
>>>>>>>>>>> sure what's the meaning
>>>>>>>>>>
>>>>>>>>>> i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is
>>>>>>>>>> acting like a cache for easing ATB data load, by providing h/w
>>>>>>>>>> buffering.
>>>>>>>>>> (In your case, it may not be providing any buffering, it doesn't
>>>>>>>>>> matter either way, as it is not visible to the driver).
>>>>>>>>>>
>>>>>>>>>> Suzuki
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> of this. Could you describe it in detail?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Best,
>>>>>>>>>>>
>>>>>>>>>>> Tao
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Suzuki
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>> drivers/hwtracing/coresight/coresight-core.c | 81
>>>>>>>>>>>>> ++++++++++++++++---
>>>>>>>>>>>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>>>>>>>>>>>>> include/linux/coresight.h | 2 +
>>>>>>>>>>>>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>>>>>>>>>>>>
>>>>>>>>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>>>> b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>>>> index 5dde597403b3..b1b5e6d9ec7a 100644
>>>>>>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>>>>>>>>>>> @@ -113,15 +113,63 @@ struct coresight_device
>>>>>>>>>>>>> *coresight_get_percpu_sink(int cpu)
>>>>>>>>>>>>> }
>>>>>>>>>>>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>>>>>>>>>>>> +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;
>>>>>>>>>>>>> +}
>>>>>>>>>>>>> +
>>>>>>>>>>>>> +/**
>>>>>>>>>>>>> + * 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_label == NULL)
>>>>>>>>>>>>> + return ret;
>>>>>>>>>>>>> +
>>>>>>>>>>>>> + source = coresight_get_source(path);
>>>>>>>>>>>>> + if (source == NULL)
>>>>>>>>>>>>> + return ret;
>>>>>>>>>>>>> +
>>>>>>>>>>>>> + if (strstr(kobject_get_path(&source->dev.kobj,
>>>>>>>>>>>>> GFP_KERNEL),
>>>>>>>>>>>>> + conn->source_label))
>>>>>>>>>>>>> + ret = 0;
>>>>>>>>>>>>> + else
>>>>>>>>>>>>> + ret = -1;
>>>>>>>>>>>>> +
>>>>>>>>>>>>> + return ret;
>>>>>>>>>>>>> +}
>>>>>>>>>>>>> +
>>>>>>>>>>>>> static struct coresight_connection *
>>>>>>>>>>>>> coresight_find_out_connection(struct coresight_device
>>>>>>>>>>>>> *src_dev,
>>>>>>>>>>>>> - struct coresight_device *dest_dev)
>>>>>>>>>>>>> + struct coresight_device *dest_dev,
>>>>>>>>>>>>> + struct list_head *path)
>>>>>>>>>>>>> {
>>>>>>>>>>>>> int i;
>>>>>>>>>>>>> struct coresight_connection *conn;
>>>>>>>>>>>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>>>>>>>>>>> conn = src_dev->pdata->out_conns[i];
>>>>>>>>>>>>> + if (coresight_source_filter(path, conn))
>>>>>>>>>>>>> + continue;
>>>>>>>>>>>>> if (conn->dest_dev == dest_dev)
>>>>>>>>>>>>> return conn;
>>>>>>>>>>>>> }
>>>>>>>>>>>>> @@ -312,7 +360,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;
>>>>>>>>>>>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>>>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>>>> if (!parent || !child)
>>>>>>>>>>>>> return -EINVAL;
>>>>>>>>>>>>> - inconn = coresight_find_out_connection(parent,
>>>>>>>>>>>>> csdev);
>>>>>>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev,
>>>>>>>>>>>>> path);
>>>>>>>>>>>>> + outconn = coresight_find_out_connection(csdev, child,
>>>>>>>>>>>>> + path);
>>>>>>>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>>>>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG
>>>>>>>>>>>>> &&
>>>>>>>>>>>>> IS_ERR(inconn))
>>>>>>>>>>>>> @@ -341,7 +390,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;
>>>>>>>>>>>>> int link_subtype;
>>>>>>>>>>>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>>>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>>>> if (!parent || !child)
>>>>>>>>>>>>> return;
>>>>>>>>>>>>> - inconn = coresight_find_out_connection(parent,
>>>>>>>>>>>>> csdev);
>>>>>>>>>>>>> - outconn = coresight_find_out_connection(csdev, child);
>>>>>>>>>>>>> + inconn = coresight_find_out_connection(parent, csdev,
>>>>>>>>>>>>> path);
>>>>>>>>>>>>> + outconn = coresight_find_out_connection(csdev, child,
>>>>>>>>>>>>> + path);
>>>>>>>>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>>>>>>>>> if (link_ops(csdev)->disable) { @@ -507,7 +557,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;
>>>>>>>>>>>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head
>>>>>>>>>>>>> *path, enum cs_mode mode,
>>>>>>>>>>>>> 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;
>>>>>>>>>>>>> @@ -802,7 +852,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;
>>>>>>>>>>>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>>>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>>>> if (coresight_is_percpu_source(csdev) &&
>>>>>>>>>>>>> coresight_is_percpu_sink(sink) &&
>>>>>>>>>>>>> sink == per_cpu(csdev_sink,
>>>>>>>>>>>>> source_ops(csdev)->cpu_id(csdev))) {
>>>>>>>>>>>>> - if (_coresight_build_path(sink, sink, path) == 0) {
>>>>>>>>>>>>> + if (_coresight_build_path(sink, sink, path, source)
>>>>>>>>>>>>> + == 0) {
>>>>>>>>>>>>> found = true;
>>>>>>>>>>>>> goto out;
>>>>>>>>>>>>> }
>>>>>>>>>>>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>>>>>>>>>>>>> coresight_device *csdev,
>>>>>>>>>>>>> struct coresight_device *child_dev;
>>>>>>>>>>>>> child_dev =
>>>>>>>>>>>>> csdev->pdata->out_conns[i]->dest_dev;
>>>>>>>>>>>>> + if (csdev->pdata->out_conns[i]->source_label &&
>>>>>>>>>>>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>>>>>>>>>>>> + csdev->pdata->out_conns[i]->source_label))
>>>>>>>>>>>>> + continue;
>>>>>>>>>>>>> if (child_dev &&
>>>>>>>>>>>>> - _coresight_build_path(child_dev, sink, path)
>>>>>>>>>>>>> == 0) {
>>>>>>>>>>>>> + _coresight_build_path(child_dev, sink, path,
>>>>>>>>>>>>> + source)
>>>>>>>>>>>>> == 0) {
>>>>>>>>>>>>> found = true;
>>>>>>>>>>>>> break;
>>>>>>>>>>>>> }
>>>>>>>>>>>>> @@ -871,7 +926,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 9d550f5697fa..f553fb20966d 100644
>>>>>>>>>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>>>>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>>>>>>>>>>> @@ -205,6 +205,7 @@ static int
>>>>>>>>>>>>> of_coresight_parse_endpoint(struct
>>>>>>>>>>>>> device *dev,
>>>>>>>>>>>>> struct fwnode_handle *rdev_fwnode;
>>>>>>>>>>>>> struct coresight_connection conn = {};
>>>>>>>>>>>>> struct coresight_connection *new_conn;
>>>>>>>>>>>>> + const char *label;
>>>>>>>>>>>>> do {
>>>>>>>>>>>>> /* Parse the local port details */ @@ -243,6
>>>>>>>>>>>>> +244,10 @@ static int of_coresight_parse_endpoint(struct
>>>>>>>>>>>>> device *dev,
>>>>>>>>>>>>> conn.dest_fwnode =
>>>>>>>>>>>>> fwnode_handle_get(rdev_fwnode);
>>>>>>>>>>>>> conn.dest_port = rendpoint.port;
>>>>>>>>>>>>> + conn.source_label = NULL;
>>>>>>>>>>>>> + if (!of_property_read_string(ep, "label", &label))
>>>>>>>>>>>>> + conn.source_label = label;
>>>>>>>>>>>>> +
>>>>>>>>>>>>> new_conn = coresight_add_out_conn(dev, pdata,
>>>>>>>>>>>>> &conn);
>>>>>>>>>>>>> if (IS_ERR_VALUE(new_conn)) {
>>>>>>>>>>>>> fwnode_handle_put(conn.dest_fwnode);
>>>>>>>>>>>>> diff --git a/include/linux/coresight.h
>>>>>>>>>>>>> b/include/linux/coresight.h index e8b6e388218c..a9c06ef9bbb2
>>>>>>>>>>>>> 100644
>>>>>>>>>>>>> --- a/include/linux/coresight.h
>>>>>>>>>>>>> +++ b/include/linux/coresight.h
>>>>>>>>>>>>> @@ -167,6 +167,7 @@ struct coresight_desc {
>>>>>>>>>>>>> * struct coresight_connection - representation of a
>>>>>>>>>>>>> single connection
>>>>>>>>>>>>> * @src_port: a connection's output port number.
>>>>>>>>>>>>> * @dest_port: destination's input port number
>>>>>>>>>>>>> @src_port is
>>>>>>>>>>>>> connected to.
>>>>>>>>>>>>> + * @source_label: source component's label.
>>>>>>>>>>>>> * @dest_fwnode: destination component's fwnode handle.
>>>>>>>>>>>>> * @dest_dev: a @coresight_device representation of
>>>>>>>>>>>>> the
>>>> component
>>>>>>>>>>>>> connected to @src_port. NULL until the device is
>>>>>>>>>>>>> created @@ -195,6 +196,7 @@ struct coresight_desc {
>>>>>>>>>>>>> struct coresight_connection {
>>>>>>>>>>>>> int src_port;
>>>>>>>>>>>>> int dest_port;
>>>>>>>>>>>>> + const char *source_label;
>>>>>>>>>>>>> struct fwnode_handle *dest_fwnode;
>>>>>>>>>>>>> struct coresight_device *dest_dev;
>>>>>>>>>>>>> struct coresight_sysfs_link *link;
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> 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]
>>
>
--
Thanks,
Tingwei