* [PATCH v5 0/4] source filtering for multi-port output
@ 2024-10-30 9:32 Tao Zhang
2024-10-30 9:32 ` [PATCH v5 1/4] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering Tao Zhang
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Tao Zhang @ 2024-10-30 9:32 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: Tao Zhang, coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
In our hardware design, by combining a funnel and a replicator, it
implement a hardware device with one-to-one correspondence between
output ports and input ports. The programming usage on this device
is the same as funnel. The software uses a funnel and a static
replicator to implement the driver of this device. Since original
funnels only support a single output connection and original
replicator only support a single input connection, the code needs
to be modified to support this new feature. The following is a
typical topology diagram of multi-port output mechanism.
|----------| |---------| |----------| |---------|
| TPDM 0 | | Source0 | | Source 1 | | TPDM 1 |
|----------| |---------| |----------| |---------|
| | | |
| | | |
| --------- | | |
| | | |
| | | |
| | | |
\-------------/ ---------------------- |
\ Funnel 0 / | |
----------- | ------------------------------
| | |
| | |
\------------------/
\ Funnel 1 / ----|
\--------------/ |
| |----> Combine a funnel and a
| | static replicator
/-----------------\ |
/ replicator 0 \ ----|
/---------------------\
| | |
| | |-----------|
| |---------| |
| |TPDM0 |TPDM1
| \-----------------/
| \ TPDA 0 /
| \-------------/
| |
| |
|Source0/1 |
\-------------------------------/
\ Funnel 2 /
\---------------------------/
Changes in V5:
1. Replace "filter-src" with "filter-source" in the
dt-binding document.
-- Suzuki K Poulose
2. Optimize the comments of the patch "coresight:
Add support for trace filtering by source" due to bad
example.
-- Suzuki K Poulose
3. Correct spelling errors in the patch "coresight:
Add support for trace filtering by source".
-- Suzuki K Poulose
4. Optimize the function "coresight_blocks_source".
-- Suzuki K Poulose
5. Add { } in the function "of_coresight_parse_endpoint".
-- Suzuki K Poulose
6. Adjust the order of the patches.
-- Suzuki K Poulose
7. Adjust the alignment in "coresight-platform.c".
-- Suzuki K Poulose
Changes in V4:
1. Use "coresight_get_source(path)" in the function
"coresight_disable_path_from" instead of explicitly
passing the source.
-- Suzuki K Poulose
2. Optimize the order of the input parameters for
"_coresight_build_path".
-- Suzuki K Poulose
3. Reuse the method "coresight_block_source" in
"_coresight_build_path".
-- Suzuki K Poulose
4. Remove the unnecessary () in "coresight_build_path".
-- Suzuki K Poulose
5. Add a helper to check if a device is SOURCE.
-- Suzuki K Poulose
6. Adjust the posistion of setting "still_orphan" in
"coresight_build_path".
-- Suzuki K Poulose
Changes in V3:
1. Rename the function "coresight_source_filter" to
"coresight_block_source". And refine this function.
-- Suzuki K Poulose
2. Rename the parameters of the function
"coresight_find_out_connection" to avoid confusion.
-- Suzuki K Poulose
3. Get the source of path in "coresight_enable_path" and
"coresight_disable_path".
-- Suzuki K Poulose
4. Fix filter source device before skip the port in
"coresight_orphan_match".
-- Suzuki K Poulose
5. Make sure the device still orphan if whter is a filter
source firmware node but the filter source device is null.
-- Suzuki K Poulose
6. Walk through the entire coresight bus and fixup the
"filter_src_dev" if the source is being removed.
-- Suzuki K Poulose
7. Refine the commit description of patch#2.
-- Suzuki K Poulose
8. Fix the warning reported by kernel test robot.
-- kernel test robot.
9. Use the source device directly if the port has a
hardcoded filter in "tpda_get_element_size".
-- Suzuki K Poulose
Changes in V2:
1. Change the reference for endpoint property in dt-binding.
-- Krzysztof Kozlowski
2. Change the property name "filter_src" to "filter-src".
-- Krzysztof Kozlowski
3. Fix the errors in running 'make dt_binding_check'.
-- Rob Herring
4. Pass in the source parameter instead of path.
-- Suzuki K Poulose
5. Reset the "filter_src_dev" if the "src" csdev is being removed.
-- Suzuki K Poulose
6. Add a warning if the "filter_src_dev" is of not the
type DEV_TYPE_SOURCE.
-- Suzuki K Poulose
7. Optimize the procedure for handling all possible cases.
-- Suzuki K Poulose
Changes in V1:
1. Add a static replicator connect to a funnel to implement the
correspondence between the output ports and the input ports on
funnels.
-- Suzuki K Poulose
2. Add filter_src_dev and filter_src_dev phandle to
"coresight_connection" struct, and populate them if there is one.
-- Suzuki K Poulose
3. To look at the phandle and then fixup/remove the filter_src
device in fixup/remove connections.
-- Suzuki K Poulose
4. When TPDA reads DSB/CMB element size, it is implemented by
looking up filter src device in the connections.
-- Suzuki K Poulose
Tao Zhang (4):
dt-bindings: arm: qcom,coresight-static-replicator: Add property for
source filtering
coresight: Add a helper to check if a device is source
coresight: Add support for trace filtering by source
coresight-tpda: Optimize the function of reading element size
.../arm/arm,coresight-static-replicator.yaml | 19 ++-
drivers/hwtracing/coresight/coresight-core.c | 113 +++++++++++++++---
.../hwtracing/coresight/coresight-platform.c | 18 +++
drivers/hwtracing/coresight/coresight-tpda.c | 13 +-
include/linux/coresight.h | 12 +-
5 files changed, 152 insertions(+), 23 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 1/4] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering
2024-10-30 9:32 [PATCH v5 0/4] source filtering for multi-port output Tao Zhang
@ 2024-10-30 9:32 ` Tao Zhang
2024-10-30 9:32 ` [PATCH v5 2/4] coresight: Add a helper to check if a device is source Tao Zhang
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Tao Zhang @ 2024-10-30 9:32 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: Tao Zhang, coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
The is some "magic" hard coded filtering in the replicators,
which only passes through trace from a particular "source". Add
a new property "filter-source" to label a phandle to the coresight
trace source device matching the hard coded filtering for the port.
Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../arm/arm,coresight-static-replicator.yaml | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
index 1892a091ac35..a6f793ea03b6 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
@@ -45,7 +45,22 @@ properties:
patternProperties:
'^port@[01]$':
description: Output connections to CoreSight Trace bus
- $ref: /schemas/graph.yaml#/properties/port
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+
+ properties:
+ endpoint:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ unevaluatedProperties: false
+
+ properties:
+ filter-source:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ phandle to the coresight trace source device matching the
+ hard coded filtering for this port
+
+ remote-endpoint: true
required:
- compatible
@@ -72,6 +87,7 @@ examples:
reg = <0>;
replicator_out_port0: endpoint {
remote-endpoint = <&etb_in_port>;
+ filter-source = <&tpdm_video>;
};
};
@@ -79,6 +95,7 @@ examples:
reg = <1>;
replicator_out_port1: endpoint {
remote-endpoint = <&tpiu_in_port>;
+ filter-source = <&tpdm_mdss>;
};
};
};
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 2/4] coresight: Add a helper to check if a device is source
2024-10-30 9:32 [PATCH v5 0/4] source filtering for multi-port output Tao Zhang
2024-10-30 9:32 ` [PATCH v5 1/4] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering Tao Zhang
@ 2024-10-30 9:32 ` Tao Zhang
2024-10-30 9:32 ` [PATCH v5 3/4] coresight: Add support for trace filtering by source Tao Zhang
2024-10-30 9:32 ` [PATCH v5 4/4] coresight-tpda: Optimize the function of reading element size Tao Zhang
3 siblings, 0 replies; 11+ messages in thread
From: Tao Zhang @ 2024-10-30 9:32 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: Tao Zhang, coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
Since there are a lot of places in the code to check whether the
device is source, add a helper to check it.
Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
drivers/hwtracing/coresight/coresight-tpda.c | 2 +-
include/linux/coresight.h | 7 ++++++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index bfca103f9f84..ad023a2a99d1 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -24,7 +24,7 @@ DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
static bool coresight_device_is_tpdm(struct coresight_device *csdev)
{
- return (csdev->type == CORESIGHT_DEV_TYPE_SOURCE) &&
+ return (coresight_is_device_source(csdev)) &&
(csdev->subtype.source_subtype ==
CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM);
}
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index c13342594278..9311df8538fc 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -588,9 +588,14 @@ static inline void csdev_access_write64(struct csdev_access *csa, u64 val, u32 o
}
#endif /* CONFIG_64BIT */
+static inline bool coresight_is_device_source(struct coresight_device *csdev)
+{
+ return csdev && (csdev->type == CORESIGHT_DEV_TYPE_SOURCE);
+}
+
static inline bool coresight_is_percpu_source(struct coresight_device *csdev)
{
- return csdev && (csdev->type == CORESIGHT_DEV_TYPE_SOURCE) &&
+ return csdev && coresight_is_device_source(csdev) &&
(csdev->subtype.source_subtype == CORESIGHT_DEV_SUBTYPE_SOURCE_PROC);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 3/4] coresight: Add support for trace filtering by source
2024-10-30 9:32 [PATCH v5 0/4] source filtering for multi-port output Tao Zhang
2024-10-30 9:32 ` [PATCH v5 1/4] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering Tao Zhang
2024-10-30 9:32 ` [PATCH v5 2/4] coresight: Add a helper to check if a device is source Tao Zhang
@ 2024-10-30 9:32 ` Tao Zhang
2024-11-13 13:39 ` Suzuki K Poulose
2024-10-30 9:32 ` [PATCH v5 4/4] coresight-tpda: Optimize the function of reading element size Tao Zhang
3 siblings, 1 reply; 11+ messages in thread
From: Tao Zhang @ 2024-10-30 9:32 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: Tao Zhang, coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
Some replicators have hard coded filtering of "trace" data, based on the
source device. This is different from the trace filtering based on
TraceID, available in the standard programmable replicators. e.g.,
Qualcomm replicators have filtering based on custom trace protocol
format and is not programmable.
The source device could be connected to the replicator via intermediate
components (e.g., a funnel). Thus we need platform information from
the firmware tables to decide the source device corresponding to a
given output port from the replicator. Given this affects "trace
path building" and traversing the path back from the sink to source,
add the concept of "filtering by source" to the generic coresight
connection.
The specified source will be marked like below in the Devicetree.
test-replicator {
... ... ... ...
out-ports {
... ... ... ...
port@0 {
reg = <0>;
xyz: endpoint {
remote-endpoint = <&zyx>;
filter-source = <&source_1>; <-- To specify the source to
}; be filtered out here.
};
port@1 {
reg = <1>;
abc: endpoint {
remote-endpoint = <&cba>;
filter-source = <&source_2>; <-- To specify the source to
}; be filtered out here.
};
};
};
Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
drivers/hwtracing/coresight/coresight-core.c | 113 +++++++++++++++---
.../hwtracing/coresight/coresight-platform.c | 18 +++
include/linux/coresight.h | 5 +
3 files changed, 117 insertions(+), 19 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index ea38ecf26fcb..0a9380350fb5 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -75,22 +75,54 @@ 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 (!coresight_is_device_source(csdev))
+ return NULL;
+
+ return csdev;
+}
+
+/**
+ * coresight_blocks_source - checks whether the connection matches the source
+ * of path if connection is bound to specific source.
+ * @src: The source device of the trace path
+ * @conn: The connection of one outport
+ *
+ * Return false if the connection doesn't have a source binded or source of the
+ * path matches the source binds to connection.
+ */
+static bool coresight_blocks_source(struct coresight_device *src,
+ struct coresight_connection *conn)
+{
+ return conn->filter_src_fwnode && (conn->filter_src_dev != src);
+}
+
static struct coresight_connection *
-coresight_find_out_connection(struct coresight_device *src_dev,
- struct coresight_device *dest_dev)
+coresight_find_out_connection(struct coresight_device *csdev,
+ struct coresight_device *out_dev,
+ struct coresight_device *trace_src)
{
int i;
struct coresight_connection *conn;
- for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
- conn = src_dev->pdata->out_conns[i];
- if (conn->dest_dev == dest_dev)
+ for (i = 0; i < csdev->pdata->nr_outconns; i++) {
+ conn = csdev->pdata->out_conns[i];
+ if (coresight_blocks_source(trace_src, conn))
+ continue;
+ if (conn->dest_dev == out_dev)
return conn;
}
- dev_err(&src_dev->dev,
- "couldn't find output connection, src_dev: %s, dest_dev: %s\n",
- dev_name(&src_dev->dev), dev_name(&dest_dev->dev));
+ dev_err(&csdev->dev,
+ "couldn't find output connection, csdev: %s, out_dev: %s\n",
+ dev_name(&csdev->dev), dev_name(&out_dev->dev));
return ERR_PTR(-ENODEV);
}
@@ -251,7 +283,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 coresight_device *source)
{
int link_subtype;
struct coresight_connection *inconn, *outconn;
@@ -259,8 +292,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, source);
+ outconn = coresight_find_out_connection(csdev, child, source);
link_subtype = csdev->subtype.link_subtype;
if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && IS_ERR(inconn))
@@ -273,15 +306,16 @@ 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 coresight_device *source)
{
struct coresight_connection *inconn, *outconn;
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, source);
+ outconn = coresight_find_out_connection(csdev, child, source);
link_ops(csdev)->disable(csdev, inconn, outconn);
}
@@ -375,7 +409,8 @@ 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,
+ coresight_get_source(path));
break;
default:
break;
@@ -418,7 +453,9 @@ int coresight_enable_path(struct list_head *path, enum cs_mode mode,
u32 type;
struct coresight_node *nd;
struct coresight_device *csdev, *parent, *child;
+ struct coresight_device *source;
+ source = coresight_get_source(path);
list_for_each_entry_reverse(nd, path, link) {
csdev = nd->csdev;
type = csdev->type;
@@ -456,7 +493,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, source);
if (ret)
goto err;
break;
@@ -619,6 +656,7 @@ static void coresight_drop_device(struct coresight_device *csdev)
/**
* _coresight_build_path - recursively build a path from a @csdev to a sink.
* @csdev: The device to start from.
+ * @source: The trace source device of the path.
* @sink: The final sink we want in this path.
* @path: The list to add devices to.
*
@@ -628,6 +666,7 @@ static void coresight_drop_device(struct coresight_device *csdev)
* the source is the first device and the sink the last one.
*/
static int _coresight_build_path(struct coresight_device *csdev,
+ struct coresight_device *source,
struct coresight_device *sink,
struct list_head *path)
{
@@ -641,7 +680,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, source, sink, path) == 0) {
found = true;
goto out;
}
@@ -652,8 +691,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 (coresight_blocks_source(source, csdev->pdata->out_conns[i]))
+ continue;
+
if (child_dev &&
- _coresight_build_path(child_dev, sink, path) == 0) {
+ _coresight_build_path(child_dev, source, sink, path) == 0) {
found = true;
break;
}
@@ -698,7 +741,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, source, sink, path);
if (rc) {
kfree(path);
return ERR_PTR(rc);
@@ -927,6 +970,16 @@ static int coresight_orphan_match(struct device *dev, void *data)
for (i = 0; i < src_csdev->pdata->nr_outconns; i++) {
conn = src_csdev->pdata->out_conns[i];
+ /* Fix filter source device before skip the port */
+ if (conn->filter_src_fwnode && !conn->filter_src_dev) {
+ if (dst_csdev &&
+ (conn->filter_src_fwnode == dst_csdev->dev.fwnode) &&
+ !WARN_ON_ONCE(!coresight_is_device_source(dst_csdev)))
+ conn->filter_src_dev = dst_csdev;
+ else
+ still_orphan = true;
+ }
+
/* Skip the port if it's already connected. */
if (conn->dest_dev)
continue;
@@ -977,18 +1030,40 @@ static int coresight_fixup_orphan_conns(struct coresight_device *csdev)
csdev, coresight_orphan_match);
}
+static int coresight_clear_filter_source(struct device *dev, void *data)
+{
+ int i;
+ struct coresight_device *source = data;
+ struct coresight_device *csdev = to_coresight_device(dev);
+
+ for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
+ if (csdev->pdata->out_conns[i]->filter_src_dev == source)
+ csdev->pdata->out_conns[i]->filter_src_dev = NULL;
+ }
+ return 0;
+}
+
/* coresight_remove_conns - Remove other device's references to this device */
static void coresight_remove_conns(struct coresight_device *csdev)
{
int i, j;
struct coresight_connection *conn;
+ if (coresight_is_device_source(csdev))
+ bus_for_each_dev(&coresight_bustype, NULL, csdev,
+ coresight_clear_filter_source);
+
/*
* Remove the input connection references from the destination device
* for each output connection.
*/
for (i = 0; i < csdev->pdata->nr_outconns; i++) {
conn = csdev->pdata->out_conns[i];
+ if (conn->filter_src_fwnode) {
+ conn->filter_src_dev = NULL;
+ fwnode_handle_put(conn->filter_src_fwnode);
+ }
+
if (!conn->dest_dev)
continue;
diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
index 64e171eaad82..d5532caa9e92 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -243,6 +243,24 @@ static int of_coresight_parse_endpoint(struct device *dev,
conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
conn.dest_port = rendpoint.port;
+ /*
+ * Get the firmware node of the filter source through the
+ * reference. This could be used to filter the source in
+ * building path.
+ */
+ conn.filter_src_fwnode =
+ fwnode_find_reference(&ep->fwnode, "filter-source", 0);
+ if (IS_ERR(conn.filter_src_fwnode)) {
+ conn.filter_src_fwnode = NULL;
+ } else {
+ conn.filter_src_dev =
+ coresight_find_csdev_by_fwnode(conn.filter_src_fwnode);
+ if (conn.filter_src_dev &&
+ !coresight_is_device_source(conn.filter_src_dev))
+ dev_warn(&conn.filter_src_dev->dev,
+ "Filter source is not a source device\n");
+ }
+
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 9311df8538fc..f372c01ae2fc 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -172,6 +172,9 @@ struct coresight_desc {
* @dest_dev: a @coresight_device representation of the component
connected to @src_port. NULL until the device is created
* @link: Representation of the connection as a sysfs link.
+ * @filter_src_fwnode: filter source component's fwnode handle.
+ * @filter_src_dev: a @coresight_device representation of the component that
+ needs to be filtered.
*
* The full connection structure looks like this, where in_conns store
* references to same connection as the source device's out_conns.
@@ -200,6 +203,8 @@ struct coresight_connection {
struct coresight_device *dest_dev;
struct coresight_sysfs_link *link;
struct coresight_device *src_dev;
+ struct fwnode_handle *filter_src_fwnode;
+ struct coresight_device *filter_src_dev;
atomic_t src_refcnt;
atomic_t dest_refcnt;
};
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 4/4] coresight-tpda: Optimize the function of reading element size
2024-10-30 9:32 [PATCH v5 0/4] source filtering for multi-port output Tao Zhang
` (2 preceding siblings ...)
2024-10-30 9:32 ` [PATCH v5 3/4] coresight: Add support for trace filtering by source Tao Zhang
@ 2024-10-30 9:32 ` Tao Zhang
3 siblings, 0 replies; 11+ messages in thread
From: Tao Zhang @ 2024-10-30 9:32 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: Tao Zhang, coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
Since the new funnel device supports multi-port output scenarios,
there may be more than one TPDM connected to one TPDA. In this
way, when reading the element size of the TPDM, TPDA driver needs
to find the expected TPDM corresponding to the filter source.
When TPDA finds a TPDM or a filter source from a input connection,
it will read the Devicetree to get the expected TPDM's element
size.
Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
drivers/hwtracing/coresight/coresight-tpda.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index ad023a2a99d1..413828f19d60 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -110,6 +110,16 @@ static int tpda_get_element_size(struct tpda_drvdata *drvdata,
csdev->pdata->in_conns[i]->dest_port != inport)
continue;
+ /*
+ * If this port has a hardcoded filter, use the source
+ * device directly.
+ */
+ if (csdev->pdata->in_conns[i]->filter_src_fwnode) {
+ in = csdev->pdata->in_conns[i]->filter_src_dev;
+ if (!in)
+ continue;
+ }
+
if (coresight_device_is_tpdm(in)) {
if (drvdata->dsb_esize || drvdata->cmb_esize)
return -EEXIST;
@@ -124,7 +134,6 @@ static int tpda_get_element_size(struct tpda_drvdata *drvdata,
}
}
-
return rc;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/4] coresight: Add support for trace filtering by source
2024-10-30 9:32 ` [PATCH v5 3/4] coresight: Add support for trace filtering by source Tao Zhang
@ 2024-11-13 13:39 ` Suzuki K Poulose
2024-11-15 9:18 ` Tao Zhang
0 siblings, 1 reply; 11+ messages in thread
From: Suzuki K Poulose @ 2024-11-13 13:39 UTC (permalink / raw)
To: Tao Zhang, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
On 30/10/2024 09:32, Tao Zhang wrote:
> Some replicators have hard coded filtering of "trace" data, based on the
> source device. This is different from the trace filtering based on
> TraceID, available in the standard programmable replicators. e.g.,
> Qualcomm replicators have filtering based on custom trace protocol
> format and is not programmable.
>
> The source device could be connected to the replicator via intermediate
> components (e.g., a funnel). Thus we need platform information from
> the firmware tables to decide the source device corresponding to a
> given output port from the replicator. Given this affects "trace
> path building" and traversing the path back from the sink to source,
> add the concept of "filtering by source" to the generic coresight
> connection.
>
> The specified source will be marked like below in the Devicetree.
> test-replicator {
> ... ... ... ...
> out-ports {
> ... ... ... ...
> port@0 {
> reg = <0>;
> xyz: endpoint {
> remote-endpoint = <&zyx>;
> filter-source = <&source_1>; <-- To specify the source to
> }; be filtered out here.
> };
>
> port@1 {
> reg = <1>;
> abc: endpoint {
> remote-endpoint = <&cba>;
> filter-source = <&source_2>; <-- To specify the source to
> }; be filtered out here.
> };
> };
> };
>
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 113 +++++++++++++++---
> .../hwtracing/coresight/coresight-platform.c | 18 +++
> include/linux/coresight.h | 5 +
> 3 files changed, 117 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index ea38ecf26fcb..0a9380350fb5 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -75,22 +75,54 @@ 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 (!coresight_is_device_source(csdev))
> + return NULL;
> +
> + return csdev;
> +}
> +
> +/**
> + * coresight_blocks_source - checks whether the connection matches the source
> + * of path if connection is bound to specific source.
> + * @src: The source device of the trace path
> + * @conn: The connection of one outport
> + *
> + * Return false if the connection doesn't have a source binded or source of the
> + * path matches the source binds to connection.
> + */
> +static bool coresight_blocks_source(struct coresight_device *src,
> + struct coresight_connection *conn)
> +{
> + return conn->filter_src_fwnode && (conn->filter_src_dev != src);
> +}
> +
> static struct coresight_connection *
> -coresight_find_out_connection(struct coresight_device *src_dev,
> - struct coresight_device *dest_dev)
> +coresight_find_out_connection(struct coresight_device *csdev,
> + struct coresight_device *out_dev,
> + struct coresight_device *trace_src)
> {
> int i;
> struct coresight_connection *conn;
>
> - for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
> - conn = src_dev->pdata->out_conns[i];
> - if (conn->dest_dev == dest_dev)
> + for (i = 0; i < csdev->pdata->nr_outconns; i++) {
> + conn = csdev->pdata->out_conns[i];
> + if (coresight_blocks_source(trace_src, conn))
> + continue;
> + if (conn->dest_dev == out_dev)
> return conn;
> }
>
> - dev_err(&src_dev->dev,
> - "couldn't find output connection, src_dev: %s, dest_dev: %s\n",
> - dev_name(&src_dev->dev), dev_name(&dest_dev->dev));
> + dev_err(&csdev->dev,
> + "couldn't find output connection, csdev: %s, out_dev: %s\n",
> + dev_name(&csdev->dev), dev_name(&out_dev->dev));
>
> return ERR_PTR(-ENODEV);
> }
> @@ -251,7 +283,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 coresight_device *source)
> {
> int link_subtype;
> struct coresight_connection *inconn, *outconn;
> @@ -259,8 +292,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, source);
> + outconn = coresight_find_out_connection(csdev, child, source);
> link_subtype = csdev->subtype.link_subtype;
>
> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && IS_ERR(inconn))
> @@ -273,15 +306,16 @@ 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 coresight_device *source)
> {
> struct coresight_connection *inconn, *outconn;
>
> 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, source);
> + outconn = coresight_find_out_connection(csdev, child, source);
>
> link_ops(csdev)->disable(csdev, inconn, outconn);
> }
> @@ -375,7 +409,8 @@ 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,
> + coresight_get_source(path));
> break;
> default:
> break;
> @@ -418,7 +453,9 @@ int coresight_enable_path(struct list_head *path, enum cs_mode mode,
> u32 type;
> struct coresight_node *nd;
> struct coresight_device *csdev, *parent, *child;
> + struct coresight_device *source;
>
> + source = coresight_get_source(path);
> list_for_each_entry_reverse(nd, path, link) {
> csdev = nd->csdev;
> type = csdev->type;
> @@ -456,7 +493,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, source);
> if (ret)
> goto err;
> break;
> @@ -619,6 +656,7 @@ static void coresight_drop_device(struct coresight_device *csdev)
> /**
> * _coresight_build_path - recursively build a path from a @csdev to a sink.
> * @csdev: The device to start from.
> + * @source: The trace source device of the path.
> * @sink: The final sink we want in this path.
> * @path: The list to add devices to.
> *
> @@ -628,6 +666,7 @@ static void coresight_drop_device(struct coresight_device *csdev)
> * the source is the first device and the sink the last one.
> */
> static int _coresight_build_path(struct coresight_device *csdev,
> + struct coresight_device *source,
> struct coresight_device *sink,
> struct list_head *path)
> {
> @@ -641,7 +680,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, source, sink, path) == 0) {
> found = true;
> goto out;
> }
> @@ -652,8 +691,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 (coresight_blocks_source(source, csdev->pdata->out_conns[i]))
> + continue;
> +
> if (child_dev &&
> - _coresight_build_path(child_dev, sink, path) == 0) {
> + _coresight_build_path(child_dev, source, sink, path) == 0) {
> found = true;
> break;
> }
> @@ -698,7 +741,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, source, sink, path);
> if (rc) {
> kfree(path);
> return ERR_PTR(rc);
> @@ -927,6 +970,16 @@ static int coresight_orphan_match(struct device *dev, void *data)
> for (i = 0; i < src_csdev->pdata->nr_outconns; i++) {
> conn = src_csdev->pdata->out_conns[i];
>
> + /* Fix filter source device before skip the port */
> + if (conn->filter_src_fwnode && !conn->filter_src_dev) {
> + if (dst_csdev &&
> + (conn->filter_src_fwnode == dst_csdev->dev.fwnode) &&
> + !WARN_ON_ONCE(!coresight_is_device_source(dst_csdev)))
> + conn->filter_src_dev = dst_csdev;
> + else
> + still_orphan = true;
> + }
> +
> /* Skip the port if it's already connected. */
> if (conn->dest_dev)
> continue;
> @@ -977,18 +1030,40 @@ static int coresight_fixup_orphan_conns(struct coresight_device *csdev)
> csdev, coresight_orphan_match);
> }
>
> +static int coresight_clear_filter_source(struct device *dev, void *data)
> +{
> + int i;
> + struct coresight_device *source = data;
> + struct coresight_device *csdev = to_coresight_device(dev);
> +
> + for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
> + if (csdev->pdata->out_conns[i]->filter_src_dev == source)
> + csdev->pdata->out_conns[i]->filter_src_dev = NULL;
> + }
> + return 0;
> +}
> +
> /* coresight_remove_conns - Remove other device's references to this device */
> static void coresight_remove_conns(struct coresight_device *csdev)
> {
> int i, j;
> struct coresight_connection *conn;
>
> + if (coresight_is_device_source(csdev))
> + bus_for_each_dev(&coresight_bustype, NULL, csdev,
> + coresight_clear_filter_source);
> +
> /*
> * Remove the input connection references from the destination device
> * for each output connection.
> */
> for (i = 0; i < csdev->pdata->nr_outconns; i++) {
> conn = csdev->pdata->out_conns[i];
> + if (conn->filter_src_fwnode) {
> + conn->filter_src_dev = NULL;
> + fwnode_handle_put(conn->filter_src_fwnode);
> + }
> +
> if (!conn->dest_dev)
> continue;
>
> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
> index 64e171eaad82..d5532caa9e92 100644
> --- a/drivers/hwtracing/coresight/coresight-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> @@ -243,6 +243,24 @@ static int of_coresight_parse_endpoint(struct device *dev,
> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
> conn.dest_port = rendpoint.port;
>
> + /*
> + * Get the firmware node of the filter source through the
> + * reference. This could be used to filter the source in
> + * building path.
> + */
> + conn.filter_src_fwnode =
> + fwnode_find_reference(&ep->fwnode, "filter-source", 0);
> + if (IS_ERR(conn.filter_src_fwnode)) {
> + conn.filter_src_fwnode = NULL;
> + } else {
> + conn.filter_src_dev =
> + coresight_find_csdev_by_fwnode(conn.filter_src_fwnode);
> + if (conn.filter_src_dev &&
> + !coresight_is_device_source(conn.filter_src_dev))
> + dev_warn(&conn.filter_src_dev->dev,
> + "Filter source is not a source device\n");
Do we need to reset the conn.filter_src_fwnode ? Otherwise, we can warn
from other places too.
Suzuki
> + }
> +
> 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 9311df8538fc..f372c01ae2fc 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -172,6 +172,9 @@ struct coresight_desc {
> * @dest_dev: a @coresight_device representation of the component
> connected to @src_port. NULL until the device is created
> * @link: Representation of the connection as a sysfs link.
> + * @filter_src_fwnode: filter source component's fwnode handle.
> + * @filter_src_dev: a @coresight_device representation of the component that
> + needs to be filtered.
> *
> * The full connection structure looks like this, where in_conns store
> * references to same connection as the source device's out_conns.
> @@ -200,6 +203,8 @@ struct coresight_connection {
> struct coresight_device *dest_dev;
> struct coresight_sysfs_link *link;
> struct coresight_device *src_dev;
> + struct fwnode_handle *filter_src_fwnode;
> + struct coresight_device *filter_src_dev;
> atomic_t src_refcnt;
> atomic_t dest_refcnt;
> };
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/4] coresight: Add support for trace filtering by source
2024-11-13 13:39 ` Suzuki K Poulose
@ 2024-11-15 9:18 ` Tao Zhang
2024-11-15 9:40 ` Suzuki K Poulose
0 siblings, 1 reply; 11+ messages in thread
From: Tao Zhang @ 2024-11-15 9:18 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
On 11/13/2024 9:39 PM, Suzuki K Poulose wrote:
> On 30/10/2024 09:32, Tao Zhang wrote:
>> Some replicators have hard coded filtering of "trace" data, based on the
>> source device. This is different from the trace filtering based on
>> TraceID, available in the standard programmable replicators. e.g.,
>> Qualcomm replicators have filtering based on custom trace protocol
>> format and is not programmable.
>>
>> The source device could be connected to the replicator via intermediate
>> components (e.g., a funnel). Thus we need platform information from
>> the firmware tables to decide the source device corresponding to a
>> given output port from the replicator. Given this affects "trace
>> path building" and traversing the path back from the sink to source,
>> add the concept of "filtering by source" to the generic coresight
>> connection.
>>
>> The specified source will be marked like below in the Devicetree.
>> test-replicator {
>> ... ... ... ...
>> out-ports {
>> ... ... ... ...
>> port@0 {
>> reg = <0>;
>> xyz: endpoint {
>> remote-endpoint = <&zyx>;
>> filter-source = <&source_1>; <-- To specify the
>> source to
>> }; be filtered out here.
>> };
>>
>> port@1 {
>> reg = <1>;
>> abc: endpoint {
>> remote-endpoint = <&cba>;
>> filter-source = <&source_2>; <-- To specify the
>> source to
>> }; be filtered out here.
>> };
>> };
>> };
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> ---
>> drivers/hwtracing/coresight/coresight-core.c | 113 +++++++++++++++---
>> .../hwtracing/coresight/coresight-platform.c | 18 +++
>> include/linux/coresight.h | 5 +
>> 3 files changed, 117 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>> b/drivers/hwtracing/coresight/coresight-core.c
>> index ea38ecf26fcb..0a9380350fb5 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -75,22 +75,54 @@ 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 (!coresight_is_device_source(csdev))
>> + return NULL;
>> +
>> + return csdev;
>> +}
>> +
>> +/**
>> + * coresight_blocks_source - checks whether the connection matches
>> the source
>> + * of path if connection is bound to specific source.
>> + * @src: The source device of the trace path
>> + * @conn: The connection of one outport
>> + *
>> + * Return false if the connection doesn't have a source binded or
>> source of the
>> + * path matches the source binds to connection.
>> + */
>> +static bool coresight_blocks_source(struct coresight_device *src,
>> + struct coresight_connection *conn)
>> +{
>> + return conn->filter_src_fwnode && (conn->filter_src_dev != src);
>> +}
>> +
>> static struct coresight_connection *
>> -coresight_find_out_connection(struct coresight_device *src_dev,
>> - struct coresight_device *dest_dev)
>> +coresight_find_out_connection(struct coresight_device *csdev,
>> + struct coresight_device *out_dev,
>> + struct coresight_device *trace_src)
>> {
>> int i;
>> struct coresight_connection *conn;
>> - for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>> - conn = src_dev->pdata->out_conns[i];
>> - if (conn->dest_dev == dest_dev)
>> + for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>> + conn = csdev->pdata->out_conns[i];
>> + if (coresight_blocks_source(trace_src, conn))
>> + continue;
>> + if (conn->dest_dev == out_dev)
>> return conn;
>> }
>> - dev_err(&src_dev->dev,
>> - "couldn't find output connection, src_dev: %s, dest_dev: %s\n",
>> - dev_name(&src_dev->dev), dev_name(&dest_dev->dev));
>> + dev_err(&csdev->dev,
>> + "couldn't find output connection, csdev: %s, out_dev: %s\n",
>> + dev_name(&csdev->dev), dev_name(&out_dev->dev));
>> return ERR_PTR(-ENODEV);
>> }
>> @@ -251,7 +283,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 coresight_device *source)
>> {
>> int link_subtype;
>> struct coresight_connection *inconn, *outconn;
>> @@ -259,8 +292,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, source);
>> + outconn = coresight_find_out_connection(csdev, child, source);
>> link_subtype = csdev->subtype.link_subtype;
>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>> IS_ERR(inconn))
>> @@ -273,15 +306,16 @@ 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 coresight_device *source)
>> {
>> struct coresight_connection *inconn, *outconn;
>> 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, source);
>> + outconn = coresight_find_out_connection(csdev, child, source);
>> link_ops(csdev)->disable(csdev, inconn, outconn);
>> }
>> @@ -375,7 +409,8 @@ 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,
>> + coresight_get_source(path));
>> break;
>> default:
>> break;
>> @@ -418,7 +453,9 @@ int coresight_enable_path(struct list_head *path,
>> enum cs_mode mode,
>> u32 type;
>> struct coresight_node *nd;
>> struct coresight_device *csdev, *parent, *child;
>> + struct coresight_device *source;
>> + source = coresight_get_source(path);
>> list_for_each_entry_reverse(nd, path, link) {
>> csdev = nd->csdev;
>> type = csdev->type;
>> @@ -456,7 +493,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, source);
>> if (ret)
>> goto err;
>> break;
>> @@ -619,6 +656,7 @@ static void coresight_drop_device(struct
>> coresight_device *csdev)
>> /**
>> * _coresight_build_path - recursively build a path from a @csdev
>> to a sink.
>> * @csdev: The device to start from.
>> + * @source: The trace source device of the path.
>> * @sink: The final sink we want in this path.
>> * @path: The list to add devices to.
>> *
>> @@ -628,6 +666,7 @@ static void coresight_drop_device(struct
>> coresight_device *csdev)
>> * the source is the first device and the sink the last one.
>> */
>> static int _coresight_build_path(struct coresight_device *csdev,
>> + struct coresight_device *source,
>> struct coresight_device *sink,
>> struct list_head *path)
>> {
>> @@ -641,7 +680,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, source, sink, path) == 0) {
>> found = true;
>> goto out;
>> }
>> @@ -652,8 +691,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 (coresight_blocks_source(source,
>> csdev->pdata->out_conns[i]))
>> + continue;
>> +
>> if (child_dev &&
>> - _coresight_build_path(child_dev, sink, path) == 0) {
>> + _coresight_build_path(child_dev, source, sink, path) ==
>> 0) {
>> found = true;
>> break;
>> }
>> @@ -698,7 +741,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, source, sink, path);
>> if (rc) {
>> kfree(path);
>> return ERR_PTR(rc);
>> @@ -927,6 +970,16 @@ static int coresight_orphan_match(struct device
>> *dev, void *data)
>> for (i = 0; i < src_csdev->pdata->nr_outconns; i++) {
>> conn = src_csdev->pdata->out_conns[i];
>> + /* Fix filter source device before skip the port */
>> + if (conn->filter_src_fwnode && !conn->filter_src_dev) {
>> + if (dst_csdev &&
>> + (conn->filter_src_fwnode == dst_csdev->dev.fwnode) &&
>> + !WARN_ON_ONCE(!coresight_is_device_source(dst_csdev)))
>> + conn->filter_src_dev = dst_csdev;
>> + else
>> + still_orphan = true;
>> + }
>> +
>> /* Skip the port if it's already connected. */
>> if (conn->dest_dev)
>> continue;
>> @@ -977,18 +1030,40 @@ static int coresight_fixup_orphan_conns(struct
>> coresight_device *csdev)
>> csdev, coresight_orphan_match);
>> }
>> +static int coresight_clear_filter_source(struct device *dev, void
>> *data)
>> +{
>> + int i;
>> + struct coresight_device *source = data;
>> + struct coresight_device *csdev = to_coresight_device(dev);
>> +
>> + for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
>> + if (csdev->pdata->out_conns[i]->filter_src_dev == source)
>> + csdev->pdata->out_conns[i]->filter_src_dev = NULL;
>> + }
>> + return 0;
>> +}
>> +
>> /* coresight_remove_conns - Remove other device's references to
>> this device */
>> static void coresight_remove_conns(struct coresight_device *csdev)
>> {
>> int i, j;
>> struct coresight_connection *conn;
>> + if (coresight_is_device_source(csdev))
>> + bus_for_each_dev(&coresight_bustype, NULL, csdev,
>> + coresight_clear_filter_source);
>> +
>> /*
>> * Remove the input connection references from the destination
>> device
>> * for each output connection.
>> */
>> for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>> conn = csdev->pdata->out_conns[i];
>> + if (conn->filter_src_fwnode) {
>> + conn->filter_src_dev = NULL;
>> + fwnode_handle_put(conn->filter_src_fwnode);
>> + }
>> +
>> if (!conn->dest_dev)
>> continue;
>> diff --git a/drivers/hwtracing/coresight/coresight-platform.c
>> b/drivers/hwtracing/coresight/coresight-platform.c
>> index 64e171eaad82..d5532caa9e92 100644
>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>> @@ -243,6 +243,24 @@ static int of_coresight_parse_endpoint(struct
>> device *dev,
>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>> conn.dest_port = rendpoint.port;
>> + /*
>> + * Get the firmware node of the filter source through the
>> + * reference. This could be used to filter the source in
>> + * building path.
>> + */
>> + conn.filter_src_fwnode =
>> + fwnode_find_reference(&ep->fwnode, "filter-source", 0);
>> + if (IS_ERR(conn.filter_src_fwnode)) {
>> + conn.filter_src_fwnode = NULL;
>> + } else {
>> + conn.filter_src_dev =
>> + coresight_find_csdev_by_fwnode(conn.filter_src_fwnode);
>> + if (conn.filter_src_dev &&
>> + !coresight_is_device_source(conn.filter_src_dev))
>> + dev_warn(&conn.filter_src_dev->dev,
>> + "Filter source is not a source device\n");
>
> Do we need to reset the conn.filter_src_fwnode ? Otherwise, we can
> warn from other places too.
Agree, we need to reset conn.filter_src_fwnode and conn.filter_src_dev
here if it is not a SOURCE
type device.
I will update the following code to the next patch series. Could you
help check if it is fine to you?
+ if (conn.filter_src_dev &&
+ !coresight_is_device_source(conn.filter_src_dev)) {
+ dev_warn(&conn.filter_src_dev->dev,
+ "Filter source is not a source device\n");
+ conn.filter_src_dev = NULL；
+ conn.filter_src_dev = NULL；
+ }
Best,
Tao
>
> Suzuki
>
>> + }
>> +
>> 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 9311df8538fc..f372c01ae2fc 100644
>> --- a/include/linux/coresight.h
>> +++ b/include/linux/coresight.h
>> @@ -172,6 +172,9 @@ struct coresight_desc {
>> * @dest_dev: a @coresight_device representation of the component
>> connected to @src_port. NULL until the device is created
>> * @link: Representation of the connection as a sysfs link.
>> + * @filter_src_fwnode: filter source component's fwnode handle.
>> + * @filter_src_dev: a @coresight_device representation of the
>> component that
>> + needs to be filtered.
>> *
>> * The full connection structure looks like this, where in_conns store
>> * references to same connection as the source device's out_conns.
>> @@ -200,6 +203,8 @@ struct coresight_connection {
>> struct coresight_device *dest_dev;
>> struct coresight_sysfs_link *link;
>> struct coresight_device *src_dev;
>> + struct fwnode_handle *filter_src_fwnode;
>> + struct coresight_device *filter_src_dev;
>> atomic_t src_refcnt;
>> atomic_t dest_refcnt;
>> };
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/4] coresight: Add support for trace filtering by source
2024-11-15 9:18 ` Tao Zhang
@ 2024-11-15 9:40 ` Suzuki K Poulose
2024-11-18 9:28 ` Tao Zhang
0 siblings, 1 reply; 11+ messages in thread
From: Suzuki K Poulose @ 2024-11-15 9:40 UTC (permalink / raw)
To: Tao Zhang, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
On 15/11/2024 09:18, Tao Zhang wrote:
>
> On 11/13/2024 9:39 PM, Suzuki K Poulose wrote:
>> On 30/10/2024 09:32, Tao Zhang wrote:
>>> Some replicators have hard coded filtering of "trace" data, based on the
>>> source device. This is different from the trace filtering based on
>>> TraceID, available in the standard programmable replicators. e.g.,
>>> Qualcomm replicators have filtering based on custom trace protocol
>>> format and is not programmable.
>>>
>>> The source device could be connected to the replicator via intermediate
>>> components (e.g., a funnel). Thus we need platform information from
>>> the firmware tables to decide the source device corresponding to a
>>> given output port from the replicator. Given this affects "trace
>>> path building" and traversing the path back from the sink to source,
>>> add the concept of "filtering by source" to the generic coresight
>>> connection.
>>>
>>> The specified source will be marked like below in the Devicetree.
>>> test-replicator {
>>> ... ... ... ...
>>> out-ports {
>>> ... ... ... ...
>>> port@0 {
>>> reg = <0>;
>>> xyz: endpoint {
>>> remote-endpoint = <&zyx>;
>>> filter-source = <&source_1>; <-- To specify the
>>> source to
>>> }; be filtered out here.
>>> };
>>>
>>> port@1 {
>>> reg = <1>;
>>> abc: endpoint {
>>> remote-endpoint = <&cba>;
>>> filter-source = <&source_2>; <-- To specify the
>>> source to
>>> }; be filtered out here.
>>> };
>>> };
>>> };
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>> ---
>>> drivers/hwtracing/coresight/coresight-core.c | 113 +++++++++++++++---
>>> .../hwtracing/coresight/coresight-platform.c | 18 +++
>>> include/linux/coresight.h | 5 +
>>> 3 files changed, 117 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>> b/drivers/hwtracing/coresight/coresight-core.c
>>> index ea38ecf26fcb..0a9380350fb5 100644
>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>> @@ -75,22 +75,54 @@ 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 (!coresight_is_device_source(csdev))
>>> + return NULL;
>>> +
>>> + return csdev;
>>> +}
>>> +
>>> +/**
>>> + * coresight_blocks_source - checks whether the connection matches
>>> the source
>>> + * of path if connection is bound to specific source.
>>> + * @src: The source device of the trace path
>>> + * @conn: The connection of one outport
>>> + *
>>> + * Return false if the connection doesn't have a source binded or
>>> source of the
>>> + * path matches the source binds to connection.
>>> + */
>>> +static bool coresight_blocks_source(struct coresight_device *src,
>>> + struct coresight_connection *conn)
>>> +{
>>> + return conn->filter_src_fwnode && (conn->filter_src_dev != src);
>>> +}
>>> +
>>> static struct coresight_connection *
>>> -coresight_find_out_connection(struct coresight_device *src_dev,
>>> - struct coresight_device *dest_dev)
>>> +coresight_find_out_connection(struct coresight_device *csdev,
>>> + struct coresight_device *out_dev,
>>> + struct coresight_device *trace_src)
>>> {
>>> int i;
>>> struct coresight_connection *conn;
>>> - for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>> - conn = src_dev->pdata->out_conns[i];
>>> - if (conn->dest_dev == dest_dev)
>>> + for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>>> + conn = csdev->pdata->out_conns[i];
>>> + if (coresight_blocks_source(trace_src, conn))
>>> + continue;
>>> + if (conn->dest_dev == out_dev)
>>> return conn;
>>> }
>>> - dev_err(&src_dev->dev,
>>> - "couldn't find output connection, src_dev: %s, dest_dev: %s\n",
>>> - dev_name(&src_dev->dev), dev_name(&dest_dev->dev));
>>> + dev_err(&csdev->dev,
>>> + "couldn't find output connection, csdev: %s, out_dev: %s\n",
>>> + dev_name(&csdev->dev), dev_name(&out_dev->dev));
>>> return ERR_PTR(-ENODEV);
>>> }
>>> @@ -251,7 +283,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 coresight_device *source)
>>> {
>>> int link_subtype;
>>> struct coresight_connection *inconn, *outconn;
>>> @@ -259,8 +292,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, source);
>>> + outconn = coresight_find_out_connection(csdev, child, source);
>>> link_subtype = csdev->subtype.link_subtype;
>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>> IS_ERR(inconn))
>>> @@ -273,15 +306,16 @@ 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 coresight_device *source)
>>> {
>>> struct coresight_connection *inconn, *outconn;
>>> 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, source);
>>> + outconn = coresight_find_out_connection(csdev, child, source);
>>> link_ops(csdev)->disable(csdev, inconn, outconn);
>>> }
>>> @@ -375,7 +409,8 @@ 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,
>>> + coresight_get_source(path));
>>> break;
>>> default:
>>> break;
>>> @@ -418,7 +453,9 @@ int coresight_enable_path(struct list_head *path,
>>> enum cs_mode mode,
>>> u32 type;
>>> struct coresight_node *nd;
>>> struct coresight_device *csdev, *parent, *child;
>>> + struct coresight_device *source;
>>> + source = coresight_get_source(path);
>>> list_for_each_entry_reverse(nd, path, link) {
>>> csdev = nd->csdev;
>>> type = csdev->type;
>>> @@ -456,7 +493,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, source);
>>> if (ret)
>>> goto err;
>>> break;
>>> @@ -619,6 +656,7 @@ static void coresight_drop_device(struct
>>> coresight_device *csdev)
>>> /**
>>> * _coresight_build_path - recursively build a path from a @csdev
>>> to a sink.
>>> * @csdev: The device to start from.
>>> + * @source: The trace source device of the path.
>>> * @sink: The final sink we want in this path.
>>> * @path: The list to add devices to.
>>> *
>>> @@ -628,6 +666,7 @@ static void coresight_drop_device(struct
>>> coresight_device *csdev)
>>> * the source is the first device and the sink the last one.
>>> */
>>> static int _coresight_build_path(struct coresight_device *csdev,
>>> + struct coresight_device *source,
>>> struct coresight_device *sink,
>>> struct list_head *path)
>>> {
>>> @@ -641,7 +680,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, source, sink, path) == 0) {
>>> found = true;
>>> goto out;
>>> }
>>> @@ -652,8 +691,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 (coresight_blocks_source(source,
>>> csdev->pdata->out_conns[i]))
>>> + continue;
>>> +
>>> if (child_dev &&
>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>> + _coresight_build_path(child_dev, source, sink, path) ==
>>> 0) {
>>> found = true;
>>> break;
>>> }
>>> @@ -698,7 +741,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, source, sink, path);
>>> if (rc) {
>>> kfree(path);
>>> return ERR_PTR(rc);
>>> @@ -927,6 +970,16 @@ static int coresight_orphan_match(struct device
>>> *dev, void *data)
>>> for (i = 0; i < src_csdev->pdata->nr_outconns; i++) {
>>> conn = src_csdev->pdata->out_conns[i];
>>> + /* Fix filter source device before skip the port */
>>> + if (conn->filter_src_fwnode && !conn->filter_src_dev) {
>>> + if (dst_csdev &&
>>> + (conn->filter_src_fwnode == dst_csdev->dev.fwnode) &&
>>> + !WARN_ON_ONCE(!coresight_is_device_source(dst_csdev)))
>>> + conn->filter_src_dev = dst_csdev;
>>> + else
>>> + still_orphan = true;
>>> + }
>>> +
>>> /* Skip the port if it's already connected. */
>>> if (conn->dest_dev)
>>> continue;
>>> @@ -977,18 +1030,40 @@ static int coresight_fixup_orphan_conns(struct
>>> coresight_device *csdev)
>>> csdev, coresight_orphan_match);
>>> }
>>> +static int coresight_clear_filter_source(struct device *dev, void
>>> *data)
>>> +{
>>> + int i;
>>> + struct coresight_device *source = data;
>>> + struct coresight_device *csdev = to_coresight_device(dev);
>>> +
>>> + for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
>>> + if (csdev->pdata->out_conns[i]->filter_src_dev == source)
>>> + csdev->pdata->out_conns[i]->filter_src_dev = NULL;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> /* coresight_remove_conns - Remove other device's references to
>>> this device */
>>> static void coresight_remove_conns(struct coresight_device *csdev)
>>> {
>>> int i, j;
>>> struct coresight_connection *conn;
>>> + if (coresight_is_device_source(csdev))
>>> + bus_for_each_dev(&coresight_bustype, NULL, csdev,
>>> + coresight_clear_filter_source);
>>> +
>>> /*
>>> * Remove the input connection references from the destination
>>> device
>>> * for each output connection.
>>> */
>>> for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>>> conn = csdev->pdata->out_conns[i];
>>> + if (conn->filter_src_fwnode) {
>>> + conn->filter_src_dev = NULL;
>>> + fwnode_handle_put(conn->filter_src_fwnode);
>>> + }
>>> +
>>> if (!conn->dest_dev)
>>> continue;
>>> diff --git a/drivers/hwtracing/coresight/coresight-platform.c
>>> b/drivers/hwtracing/coresight/coresight-platform.c
>>> index 64e171eaad82..d5532caa9e92 100644
>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>> @@ -243,6 +243,24 @@ static int of_coresight_parse_endpoint(struct
>>> device *dev,
>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>> conn.dest_port = rendpoint.port;
>>> + /*
>>> + * Get the firmware node of the filter source through the
>>> + * reference. This could be used to filter the source in
>>> + * building path.
>>> + */
>>> + conn.filter_src_fwnode =
>>> + fwnode_find_reference(&ep->fwnode, "filter-source", 0);
>>> + if (IS_ERR(conn.filter_src_fwnode)) {
>>> + conn.filter_src_fwnode = NULL;
>>> + } else {
>>> + conn.filter_src_dev =
>>> + coresight_find_csdev_by_fwnode(conn.filter_src_fwnode);
>>> + if (conn.filter_src_dev &&
>>> + !coresight_is_device_source(conn.filter_src_dev))
>>> + dev_warn(&conn.filter_src_dev->dev,
>>> + "Filter source is not a source device\n");
>>
>> Do we need to reset the conn.filter_src_fwnode ? Otherwise, we can
>> warn from other places too.
>
> Agree, we need to reset conn.filter_src_fwnode and conn.filter_src_dev
> here if it is not a SOURCE
>
> type device.
>
> I will update the following code to the next patch series. Could you
> help check if it is fine to you?
This looks good.
>
> + if (conn.filter_src_dev &&
> + !coresight_is_device_source(conn.filter_src_dev)) {
> + dev_warn(&conn.filter_src_dev->dev,
> + "Filter source is not a source device\n");
minor nit: It would be good to indicate which device has this
"filtering". i.e,
dev_warn(dev, "Filter source is not a trace source : %s\n",
dev_name(&conn.filter_src_dev->dev));
> + conn.filter_src_dev = NULL；
> + conn.filter_src_dev = NULL；
Duplicated line ^
Suzuki
> + }
>
> Best,
>
> Tao
>
>>
>> Suzuki
>>
>>> + }
>>> +
>>> 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 9311df8538fc..f372c01ae2fc 100644
>>> --- a/include/linux/coresight.h
>>> +++ b/include/linux/coresight.h
>>> @@ -172,6 +172,9 @@ struct coresight_desc {
>>> * @dest_dev: a @coresight_device representation of the component
>>> connected to @src_port. NULL until the device is created
>>> * @link: Representation of the connection as a sysfs link.
>>> + * @filter_src_fwnode: filter source component's fwnode handle.
>>> + * @filter_src_dev: a @coresight_device representation of the
>>> component that
>>> + needs to be filtered.
>>> *
>>> * The full connection structure looks like this, where in_conns store
>>> * references to same connection as the source device's out_conns.
>>> @@ -200,6 +203,8 @@ struct coresight_connection {
>>> struct coresight_device *dest_dev;
>>> struct coresight_sysfs_link *link;
>>> struct coresight_device *src_dev;
>>> + struct fwnode_handle *filter_src_fwnode;
>>> + struct coresight_device *filter_src_dev;
>>> atomic_t src_refcnt;
>>> atomic_t dest_refcnt;
>>> };
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/4] coresight: Add support for trace filtering by source
2024-11-15 9:40 ` Suzuki K Poulose
@ 2024-11-18 9:28 ` Tao Zhang
2024-11-18 9:56 ` Suzuki K Poulose
0 siblings, 1 reply; 11+ messages in thread
From: Tao Zhang @ 2024-11-18 9:28 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
On 11/15/2024 5:40 PM, Suzuki K Poulose wrote:
> On 15/11/2024 09:18, Tao Zhang wrote:
>>
>> On 11/13/2024 9:39 PM, Suzuki K Poulose wrote:
>>> On 30/10/2024 09:32, Tao Zhang wrote:
>>>> Some replicators have hard coded filtering of "trace" data, based
>>>> on the
>>>> source device. This is different from the trace filtering based on
>>>> TraceID, available in the standard programmable replicators. e.g.,
>>>> Qualcomm replicators have filtering based on custom trace protocol
>>>> format and is not programmable.
>>>>
>>>> The source device could be connected to the replicator via
>>>> intermediate
>>>> components (e.g., a funnel). Thus we need platform information from
>>>> the firmware tables to decide the source device corresponding to a
>>>> given output port from the replicator. Given this affects "trace
>>>> path building" and traversing the path back from the sink to source,
>>>> add the concept of "filtering by source" to the generic coresight
>>>> connection.
>>>>
>>>> The specified source will be marked like below in the Devicetree.
>>>> test-replicator {
>>>> ... ... ... ...
>>>> out-ports {
>>>> ... ... ... ...
>>>> port@0 {
>>>> reg = <0>;
>>>> xyz: endpoint {
>>>> remote-endpoint = <&zyx>;
>>>> filter-source = <&source_1>; <-- To specify the
>>>> source to
>>>> }; be filtered out here.
>>>> };
>>>>
>>>> port@1 {
>>>> reg = <1>;
>>>> abc: endpoint {
>>>> remote-endpoint = <&cba>;
>>>> filter-source = <&source_2>; <-- To specify the
>>>> source to
>>>> }; be filtered out here.
>>>> };
>>>> };
>>>> };
>>>>
>>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>>> ---
>>>> drivers/hwtracing/coresight/coresight-core.c | 113
>>>> +++++++++++++++---
>>>> .../hwtracing/coresight/coresight-platform.c | 18 +++
>>>> include/linux/coresight.h | 5 +
>>>> 3 files changed, 117 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>>> b/drivers/hwtracing/coresight/coresight-core.c
>>>> index ea38ecf26fcb..0a9380350fb5 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>> @@ -75,22 +75,54 @@ 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 (!coresight_is_device_source(csdev))
>>>> + return NULL;
>>>> +
>>>> + return csdev;
>>>> +}
>>>> +
>>>> +/**
>>>> + * coresight_blocks_source - checks whether the connection matches
>>>> the source
>>>> + * of path if connection is bound to specific source.
>>>> + * @src: The source device of the trace path
>>>> + * @conn: The connection of one outport
>>>> + *
>>>> + * Return false if the connection doesn't have a source binded or
>>>> source of the
>>>> + * path matches the source binds to connection.
>>>> + */
>>>> +static bool coresight_blocks_source(struct coresight_device *src,
>>>> + struct coresight_connection *conn)
>>>> +{
>>>> + return conn->filter_src_fwnode && (conn->filter_src_dev != src);
>>>> +}
>>>> +
>>>> static struct coresight_connection *
>>>> -coresight_find_out_connection(struct coresight_device *src_dev,
>>>> - struct coresight_device *dest_dev)
>>>> +coresight_find_out_connection(struct coresight_device *csdev,
>>>> + struct coresight_device *out_dev,
>>>> + struct coresight_device *trace_src)
>>>> {
>>>> int i;
>>>> struct coresight_connection *conn;
>>>> - for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>> - conn = src_dev->pdata->out_conns[i];
>>>> - if (conn->dest_dev == dest_dev)
>>>> + for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>>>> + conn = csdev->pdata->out_conns[i];
>>>> + if (coresight_blocks_source(trace_src, conn))
>>>> + continue;
>>>> + if (conn->dest_dev == out_dev)
>>>> return conn;
>>>> }
>>>> - dev_err(&src_dev->dev,
>>>> - "couldn't find output connection, src_dev: %s, dest_dev:
>>>> %s\n",
>>>> - dev_name(&src_dev->dev), dev_name(&dest_dev->dev));
>>>> + dev_err(&csdev->dev,
>>>> + "couldn't find output connection, csdev: %s, out_dev: %s\n",
>>>> + dev_name(&csdev->dev), dev_name(&out_dev->dev));
>>>> return ERR_PTR(-ENODEV);
>>>> }
>>>> @@ -251,7 +283,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 coresight_device *source)
>>>> {
>>>> int link_subtype;
>>>> struct coresight_connection *inconn, *outconn;
>>>> @@ -259,8 +292,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, source);
>>>> + outconn = coresight_find_out_connection(csdev, child, source);
>>>> link_subtype = csdev->subtype.link_subtype;
>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>>> IS_ERR(inconn))
>>>> @@ -273,15 +306,16 @@ 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 coresight_device *source)
>>>> {
>>>> struct coresight_connection *inconn, *outconn;
>>>> 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, source);
>>>> + outconn = coresight_find_out_connection(csdev, child, source);
>>>> link_ops(csdev)->disable(csdev, inconn, outconn);
>>>> }
>>>> @@ -375,7 +409,8 @@ 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,
>>>> + coresight_get_source(path));
>>>> break;
>>>> default:
>>>> break;
>>>> @@ -418,7 +453,9 @@ int coresight_enable_path(struct list_head
>>>> *path, enum cs_mode mode,
>>>> u32 type;
>>>> struct coresight_node *nd;
>>>> struct coresight_device *csdev, *parent, *child;
>>>> + struct coresight_device *source;
>>>> + source = coresight_get_source(path);
>>>> list_for_each_entry_reverse(nd, path, link) {
>>>> csdev = nd->csdev;
>>>> type = csdev->type;
>>>> @@ -456,7 +493,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,
>>>> source);
>>>> if (ret)
>>>> goto err;
>>>> break;
>>>> @@ -619,6 +656,7 @@ static void coresight_drop_device(struct
>>>> coresight_device *csdev)
>>>> /**
>>>> * _coresight_build_path - recursively build a path from a @csdev
>>>> to a sink.
>>>> * @csdev: The device to start from.
>>>> + * @source: The trace source device of the path.
>>>> * @sink: The final sink we want in this path.
>>>> * @path: The list to add devices to.
>>>> *
>>>> @@ -628,6 +666,7 @@ static void coresight_drop_device(struct
>>>> coresight_device *csdev)
>>>> * the source is the first device and the sink the last one.
>>>> */
>>>> static int _coresight_build_path(struct coresight_device *csdev,
>>>> + struct coresight_device *source,
>>>> struct coresight_device *sink,
>>>> struct list_head *path)
>>>> {
>>>> @@ -641,7 +680,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, source, sink, path) == 0) {
>>>> found = true;
>>>> goto out;
>>>> }
>>>> @@ -652,8 +691,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 (coresight_blocks_source(source,
>>>> csdev->pdata->out_conns[i]))
>>>> + continue;
>>>> +
>>>> if (child_dev &&
>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>>> + _coresight_build_path(child_dev, source, sink, path)
>>>> == 0) {
>>>> found = true;
>>>> break;
>>>> }
>>>> @@ -698,7 +741,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, source, sink, path);
>>>> if (rc) {
>>>> kfree(path);
>>>> return ERR_PTR(rc);
>>>> @@ -927,6 +970,16 @@ static int coresight_orphan_match(struct
>>>> device *dev, void *data)
>>>> for (i = 0; i < src_csdev->pdata->nr_outconns; i++) {
>>>> conn = src_csdev->pdata->out_conns[i];
>>>> + /* Fix filter source device before skip the port */
>>>> + if (conn->filter_src_fwnode && !conn->filter_src_dev) {
>>>> + if (dst_csdev &&
>>>> + (conn->filter_src_fwnode == dst_csdev->dev.fwnode) &&
>>>> + !WARN_ON_ONCE(!coresight_is_device_source(dst_csdev)))
>>>> + conn->filter_src_dev = dst_csdev;
>>>> + else
>>>> + still_orphan = true;
>>>> + }
>>>> +
>>>> /* Skip the port if it's already connected. */
>>>> if (conn->dest_dev)
>>>> continue;
>>>> @@ -977,18 +1030,40 @@ static int
>>>> coresight_fixup_orphan_conns(struct coresight_device *csdev)
>>>> csdev, coresight_orphan_match);
>>>> }
>>>> +static int coresight_clear_filter_source(struct device *dev,
>>>> void *data)
>>>> +{
>>>> + int i;
>>>> + struct coresight_device *source = data;
>>>> + struct coresight_device *csdev = to_coresight_device(dev);
>>>> +
>>>> + for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
>>>> + if (csdev->pdata->out_conns[i]->filter_src_dev == source)
>>>> + csdev->pdata->out_conns[i]->filter_src_dev = NULL;
>>>> + }
>>>> + return 0;
>>>> +}
>>>> +
>>>> /* coresight_remove_conns - Remove other device's references to
>>>> this device */
>>>> static void coresight_remove_conns(struct coresight_device *csdev)
>>>> {
>>>> int i, j;
>>>> struct coresight_connection *conn;
>>>> + if (coresight_is_device_source(csdev))
>>>> + bus_for_each_dev(&coresight_bustype, NULL, csdev,
>>>> + coresight_clear_filter_source);
>>>> +
>>>> /*
>>>> * Remove the input connection references from the
>>>> destination device
>>>> * for each output connection.
>>>> */
>>>> for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>>>> conn = csdev->pdata->out_conns[i];
>>>> + if (conn->filter_src_fwnode) {
>>>> + conn->filter_src_dev = NULL;
>>>> + fwnode_handle_put(conn->filter_src_fwnode);
>>>> + }
>>>> +
>>>> if (!conn->dest_dev)
>>>> continue;
>>>> diff --git a/drivers/hwtracing/coresight/coresight-platform.c
>>>> b/drivers/hwtracing/coresight/coresight-platform.c
>>>> index 64e171eaad82..d5532caa9e92 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>> @@ -243,6 +243,24 @@ static int of_coresight_parse_endpoint(struct
>>>> device *dev,
>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>>> conn.dest_port = rendpoint.port;
>>>> + /*
>>>> + * Get the firmware node of the filter source through the
>>>> + * reference. This could be used to filter the source in
>>>> + * building path.
>>>> + */
>>>> + conn.filter_src_fwnode =
>>>> + fwnode_find_reference(&ep->fwnode, "filter-source", 0);
>>>> + if (IS_ERR(conn.filter_src_fwnode)) {
>>>> + conn.filter_src_fwnode = NULL;
>>>> + } else {
>>>> + conn.filter_src_dev =
>>>> + coresight_find_csdev_by_fwnode(conn.filter_src_fwnode);
>>>> + if (conn.filter_src_dev &&
>>>> + !coresight_is_device_source(conn.filter_src_dev))
>>>> + dev_warn(&conn.filter_src_dev->dev,
>>>> + "Filter source is not a source device\n");
>>>
>>> Do we need to reset the conn.filter_src_fwnode ? Otherwise, we can
>>> warn from other places too.
>>
>> Agree, we need to reset conn.filter_src_fwnode and
>> conn.filter_src_dev here if it is not a SOURCE
>>
>> type device.
>>
>> I will update the following code to the next patch series. Could you
>> help check if it is fine to you?
>
> This looks good.
>
>>
>> + if (conn.filter_src_dev &&
>> + !coresight_is_device_source(conn.filter_src_dev)) {
>> + dev_warn(&conn.filter_src_dev->dev,
>> + "Filter source is not a source device\n");
>
> minor nit: It would be good to indicate which device has this
> "filtering". i.e,
> dev_warn(dev, "Filter source is not a trace source : %s\n",
> dev_name(&conn.filter_src_dev->dev));
Sure, I will update this to the next patch series.
>
>> + conn.filter_src_dev = NULL；
>> + conn.filter_src_dev = NULL;
Sorry for my mistake here. I also want to set "filter_src_fwnode " to
NULL if it is not a trace source.
conn.filter_src_fwnode = NULL;
Best,
Tao
>
> Duplicated line ^
>
> Suzuki
>
>> + }
>>
>> Best,
>>
>> Tao
>>
>>>
>>> Suzuki
>>>
>>>> + }
>>>> +
>>>> 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 9311df8538fc..f372c01ae2fc 100644
>>>> --- a/include/linux/coresight.h
>>>> +++ b/include/linux/coresight.h
>>>> @@ -172,6 +172,9 @@ struct coresight_desc {
>>>> * @dest_dev: a @coresight_device representation of the component
>>>> connected to @src_port. NULL until the device is created
>>>> * @link: Representation of the connection as a sysfs link.
>>>> + * @filter_src_fwnode: filter source component's fwnode handle.
>>>> + * @filter_src_dev: a @coresight_device representation of the
>>>> component that
>>>> + needs to be filtered.
>>>> *
>>>> * The full connection structure looks like this, where in_conns
>>>> store
>>>> * references to same connection as the source device's out_conns.
>>>> @@ -200,6 +203,8 @@ struct coresight_connection {
>>>> struct coresight_device *dest_dev;
>>>> struct coresight_sysfs_link *link;
>>>> struct coresight_device *src_dev;
>>>> + struct fwnode_handle *filter_src_fwnode;
>>>> + struct coresight_device *filter_src_dev;
>>>> atomic_t src_refcnt;
>>>> atomic_t dest_refcnt;
>>>> };
>>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/4] coresight: Add support for trace filtering by source
2024-11-18 9:28 ` Tao Zhang
@ 2024-11-18 9:56 ` Suzuki K Poulose
2024-11-19 8:58 ` Tao Zhang
0 siblings, 1 reply; 11+ messages in thread
From: Suzuki K Poulose @ 2024-11-18 9:56 UTC (permalink / raw)
To: Tao Zhang, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
On 18/11/2024 09:28, Tao Zhang wrote:
>
> On 11/15/2024 5:40 PM, Suzuki K Poulose wrote:
>> On 15/11/2024 09:18, Tao Zhang wrote:
>>>
>>> On 11/13/2024 9:39 PM, Suzuki K Poulose wrote:
>>>> On 30/10/2024 09:32, Tao Zhang wrote:
>>>>> Some replicators have hard coded filtering of "trace" data, based
>>>>> on the
>>>>> source device. This is different from the trace filtering based on
>>>>> TraceID, available in the standard programmable replicators. e.g.,
>>>>> Qualcomm replicators have filtering based on custom trace protocol
>>>>> format and is not programmable.
>>>>>
>>>>> The source device could be connected to the replicator via
>>>>> intermediate
>>>>> components (e.g., a funnel). Thus we need platform information from
>>>>> the firmware tables to decide the source device corresponding to a
>>>>> given output port from the replicator. Given this affects "trace
>>>>> path building" and traversing the path back from the sink to source,
>>>>> add the concept of "filtering by source" to the generic coresight
>>>>> connection.
>>>>>
>>>>> The specified source will be marked like below in the Devicetree.
>>>>> test-replicator {
>>>>> ... ... ... ...
>>>>> out-ports {
>>>>> ... ... ... ...
>>>>> port@0 {
>>>>> reg = <0>;
>>>>> xyz: endpoint {
>>>>> remote-endpoint = <&zyx>;
>>>>> filter-source = <&source_1>; <-- To specify the
>>>>> source to
>>>>> }; be filtered out here.
>>>>> };
>>>>>
>>>>> port@1 {
>>>>> reg = <1>;
>>>>> abc: endpoint {
>>>>> remote-endpoint = <&cba>;
>>>>> filter-source = <&source_2>; <-- To specify the
>>>>> source to
>>>>> }; be filtered out here.
>>>>> };
>>>>> };
>>>>> };
>>>>>
>>>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>>>> ---
>>>>> drivers/hwtracing/coresight/coresight-core.c | 113 +++++++++++++
>>>>> ++---
>>>>> .../hwtracing/coresight/coresight-platform.c | 18 +++
>>>>> include/linux/coresight.h | 5 +
>>>>> 3 files changed, 117 insertions(+), 19 deletions(-)
>>>>>
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/
>>>>> drivers/hwtracing/coresight/coresight-core.c
>>>>> index ea38ecf26fcb..0a9380350fb5 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>>> @@ -75,22 +75,54 @@ 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 (!coresight_is_device_source(csdev))
>>>>> + return NULL;
>>>>> +
>>>>> + return csdev;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * coresight_blocks_source - checks whether the connection matches
>>>>> the source
>>>>> + * of path if connection is bound to specific source.
>>>>> + * @src: The source device of the trace path
>>>>> + * @conn: The connection of one outport
>>>>> + *
>>>>> + * Return false if the connection doesn't have a source binded or
>>>>> source of the
>>>>> + * path matches the source binds to connection.
>>>>> + */
>>>>> +static bool coresight_blocks_source(struct coresight_device *src,
>>>>> + struct coresight_connection *conn)
>>>>> +{
>>>>> + return conn->filter_src_fwnode && (conn->filter_src_dev != src);
>>>>> +}
>>>>> +
>>>>> static struct coresight_connection *
>>>>> -coresight_find_out_connection(struct coresight_device *src_dev,
>>>>> - struct coresight_device *dest_dev)
>>>>> +coresight_find_out_connection(struct coresight_device *csdev,
>>>>> + struct coresight_device *out_dev,
>>>>> + struct coresight_device *trace_src)
>>>>> {
>>>>> int i;
>>>>> struct coresight_connection *conn;
>>>>> - for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>>> - conn = src_dev->pdata->out_conns[i];
>>>>> - if (conn->dest_dev == dest_dev)
>>>>> + for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>>>>> + conn = csdev->pdata->out_conns[i];
>>>>> + if (coresight_blocks_source(trace_src, conn))
>>>>> + continue;
>>>>> + if (conn->dest_dev == out_dev)
>>>>> return conn;
>>>>> }
>>>>> - dev_err(&src_dev->dev,
>>>>> - "couldn't find output connection, src_dev: %s, dest_dev:
>>>>> %s\n",
>>>>> - dev_name(&src_dev->dev), dev_name(&dest_dev->dev));
>>>>> + dev_err(&csdev->dev,
>>>>> + "couldn't find output connection, csdev: %s, out_dev: %s\n",
>>>>> + dev_name(&csdev->dev), dev_name(&out_dev->dev));
>>>>> return ERR_PTR(-ENODEV);
>>>>> }
>>>>> @@ -251,7 +283,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 coresight_device *source)
>>>>> {
>>>>> int link_subtype;
>>>>> struct coresight_connection *inconn, *outconn;
>>>>> @@ -259,8 +292,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, source);
>>>>> + outconn = coresight_find_out_connection(csdev, child, source);
>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>>>> IS_ERR(inconn))
>>>>> @@ -273,15 +306,16 @@ 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 coresight_device *source)
>>>>> {
>>>>> struct coresight_connection *inconn, *outconn;
>>>>> 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, source);
>>>>> + outconn = coresight_find_out_connection(csdev, child, source);
>>>>> link_ops(csdev)->disable(csdev, inconn, outconn);
>>>>> }
>>>>> @@ -375,7 +409,8 @@ 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,
>>>>> + coresight_get_source(path));
>>>>> break;
>>>>> default:
>>>>> break;
>>>>> @@ -418,7 +453,9 @@ int coresight_enable_path(struct list_head
>>>>> *path, enum cs_mode mode,
>>>>> u32 type;
>>>>> struct coresight_node *nd;
>>>>> struct coresight_device *csdev, *parent, *child;
>>>>> + struct coresight_device *source;
>>>>> + source = coresight_get_source(path);
>>>>> list_for_each_entry_reverse(nd, path, link) {
>>>>> csdev = nd->csdev;
>>>>> type = csdev->type;
>>>>> @@ -456,7 +493,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,
>>>>> source);
>>>>> if (ret)
>>>>> goto err;
>>>>> break;
>>>>> @@ -619,6 +656,7 @@ static void coresight_drop_device(struct
>>>>> coresight_device *csdev)
>>>>> /**
>>>>> * _coresight_build_path - recursively build a path from a @csdev
>>>>> to a sink.
>>>>> * @csdev: The device to start from.
>>>>> + * @source: The trace source device of the path.
>>>>> * @sink: The final sink we want in this path.
>>>>> * @path: The list to add devices to.
>>>>> *
>>>>> @@ -628,6 +666,7 @@ static void coresight_drop_device(struct
>>>>> coresight_device *csdev)
>>>>> * the source is the first device and the sink the last one.
>>>>> */
>>>>> static int _coresight_build_path(struct coresight_device *csdev,
>>>>> + struct coresight_device *source,
>>>>> struct coresight_device *sink,
>>>>> struct list_head *path)
>>>>> {
>>>>> @@ -641,7 +680,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, source, sink, path) == 0) {
>>>>> found = true;
>>>>> goto out;
>>>>> }
>>>>> @@ -652,8 +691,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 (coresight_blocks_source(source, csdev->pdata-
>>>>> >out_conns[i]))
>>>>> + continue;
>>>>> +
>>>>> if (child_dev &&
>>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>>>> + _coresight_build_path(child_dev, source, sink, path)
>>>>> == 0) {
>>>>> found = true;
>>>>> break;
>>>>> }
>>>>> @@ -698,7 +741,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, source, sink, path);
>>>>> if (rc) {
>>>>> kfree(path);
>>>>> return ERR_PTR(rc);
>>>>> @@ -927,6 +970,16 @@ static int coresight_orphan_match(struct
>>>>> device *dev, void *data)
>>>>> for (i = 0; i < src_csdev->pdata->nr_outconns; i++) {
>>>>> conn = src_csdev->pdata->out_conns[i];
>>>>> + /* Fix filter source device before skip the port */
>>>>> + if (conn->filter_src_fwnode && !conn->filter_src_dev) {
>>>>> + if (dst_csdev &&
>>>>> + (conn->filter_src_fwnode == dst_csdev->dev.fwnode) &&
>>>>> + !WARN_ON_ONCE(!coresight_is_device_source(dst_csdev)))
>>>>> + conn->filter_src_dev = dst_csdev;
>>>>> + else
>>>>> + still_orphan = true;
>>>>> + }
>>>>> +
>>>>> /* Skip the port if it's already connected. */
>>>>> if (conn->dest_dev)
>>>>> continue;
>>>>> @@ -977,18 +1030,40 @@ static int
>>>>> coresight_fixup_orphan_conns(struct coresight_device *csdev)
>>>>> csdev, coresight_orphan_match);
>>>>> }
>>>>> +static int coresight_clear_filter_source(struct device *dev,
>>>>> void *data)
>>>>> +{
>>>>> + int i;
>>>>> + struct coresight_device *source = data;
>>>>> + struct coresight_device *csdev = to_coresight_device(dev);
>>>>> +
>>>>> + for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
>>>>> + if (csdev->pdata->out_conns[i]->filter_src_dev == source)
>>>>> + csdev->pdata->out_conns[i]->filter_src_dev = NULL;
>>>>> + }
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> /* coresight_remove_conns - Remove other device's references to
>>>>> this device */
>>>>> static void coresight_remove_conns(struct coresight_device *csdev)
>>>>> {
>>>>> int i, j;
>>>>> struct coresight_connection *conn;
>>>>> + if (coresight_is_device_source(csdev))
>>>>> + bus_for_each_dev(&coresight_bustype, NULL, csdev,
>>>>> + coresight_clear_filter_source);
>>>>> +
>>>>> /*
>>>>> * Remove the input connection references from the
>>>>> destination device
>>>>> * for each output connection.
>>>>> */
>>>>> for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>>>>> conn = csdev->pdata->out_conns[i];
>>>>> + if (conn->filter_src_fwnode) {
>>>>> + conn->filter_src_dev = NULL;
>>>>> + fwnode_handle_put(conn->filter_src_fwnode);
>>>>> + }
>>>>> +
>>>>> if (!conn->dest_dev)
>>>>> continue;
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/
>>>>> drivers/hwtracing/coresight/coresight-platform.c
>>>>> index 64e171eaad82..d5532caa9e92 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>>> @@ -243,6 +243,24 @@ static int of_coresight_parse_endpoint(struct
>>>>> device *dev,
>>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>>>> conn.dest_port = rendpoint.port;
>>>>> + /*
>>>>> + * Get the firmware node of the filter source through the
>>>>> + * reference. This could be used to filter the source in
>>>>> + * building path.
>>>>> + */
>>>>> + conn.filter_src_fwnode =
>>>>> + fwnode_find_reference(&ep->fwnode, "filter-source", 0);
>>>>> + if (IS_ERR(conn.filter_src_fwnode)) {
>>>>> + conn.filter_src_fwnode = NULL;
>>>>> + } else {
>>>>> + conn.filter_src_dev =
>>>>> + coresight_find_csdev_by_fwnode(conn.filter_src_fwnode);
>>>>> + if (conn.filter_src_dev &&
>>>>> + !coresight_is_device_source(conn.filter_src_dev))
>>>>> + dev_warn(&conn.filter_src_dev->dev,
>>>>> + "Filter source is not a source device\n");
>>>>
>>>> Do we need to reset the conn.filter_src_fwnode ? Otherwise, we can
>>>> warn from other places too.
>>>
>>> Agree, we need to reset conn.filter_src_fwnode and
>>> conn.filter_src_dev here if it is not a SOURCE
>>>
>>> type device.
>>>
>>> I will update the following code to the next patch series. Could you
>>> help check if it is fine to you?
>>
>> This looks good.
>>
>>>
>>> + if (conn.filter_src_dev &&
>>> + !coresight_is_device_source(conn.filter_src_dev)) {
>>> + dev_warn(&conn.filter_src_dev->dev,
>>> + "Filter source is not a source device\n");
>>
>> minor nit: It would be good to indicate which device has this
>> "filtering". i.e,
>> dev_warn(dev, "Filter source is not a trace source : %s\n",
>> dev_name(&conn.filter_src_dev->dev));
> Sure, I will update this to the next patch series.
Also, it may be helpful, if we specify the "port number" which has this
wrong handle. I guess something like:
dev_warn(dev, "port %d: Filter handle is not a trace source : %s\n",
conn.src_port, dev_name(&conn.filter_src_dev->dev));
Suzuki
>>
>>> + conn.filter_src_dev = NULL；
>>> + conn.filter_src_dev = NULL;
>
> Sorry for my mistake here. I also want to set "filter_src_fwnode " to
> NULL if it is not a trace source.
>
> conn.filter_src_fwnode = NULL;
>
>
> Best,
>
> Tao
>
>>
>> Duplicated line ^
>>
>> Suzuki
>>
>>> + }
>>>
>>> Best,
>>>
>>> Tao
>>>
>>>>
>>>> Suzuki
>>>>
>>>>> + }
>>>>> +
>>>>> 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 9311df8538fc..f372c01ae2fc 100644
>>>>> --- a/include/linux/coresight.h
>>>>> +++ b/include/linux/coresight.h
>>>>> @@ -172,6 +172,9 @@ struct coresight_desc {
>>>>> * @dest_dev: a @coresight_device representation of the component
>>>>> connected to @src_port. NULL until the device is created
>>>>> * @link: Representation of the connection as a sysfs link.
>>>>> + * @filter_src_fwnode: filter source component's fwnode handle.
>>>>> + * @filter_src_dev: a @coresight_device representation of the
>>>>> component that
>>>>> + needs to be filtered.
>>>>> *
>>>>> * The full connection structure looks like this, where in_conns
>>>>> store
>>>>> * references to same connection as the source device's out_conns.
>>>>> @@ -200,6 +203,8 @@ struct coresight_connection {
>>>>> struct coresight_device *dest_dev;
>>>>> struct coresight_sysfs_link *link;
>>>>> struct coresight_device *src_dev;
>>>>> + struct fwnode_handle *filter_src_fwnode;
>>>>> + struct coresight_device *filter_src_dev;
>>>>> atomic_t src_refcnt;
>>>>> atomic_t dest_refcnt;
>>>>> };
>>>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/4] coresight: Add support for trace filtering by source
2024-11-18 9:56 ` Suzuki K Poulose
@ 2024-11-19 8:58 ` Tao Zhang
0 siblings, 0 replies; 11+ messages in thread
From: Tao Zhang @ 2024-11-19 8:58 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Mathieu Poirier, Leo Yan,
Alexander Shishkin
Cc: coresight, linux-arm-kernel, linux-kernel, devicetree,
linux-arm-msm
On 11/18/2024 5:56 PM, Suzuki K Poulose wrote:
> On 18/11/2024 09:28, Tao Zhang wrote:
>>
>> On 11/15/2024 5:40 PM, Suzuki K Poulose wrote:
>>> On 15/11/2024 09:18, Tao Zhang wrote:
>>>>
>>>> On 11/13/2024 9:39 PM, Suzuki K Poulose wrote:
>>>>> On 30/10/2024 09:32, Tao Zhang wrote:
>>>>>> Some replicators have hard coded filtering of "trace" data, based
>>>>>> on the
>>>>>> source device. This is different from the trace filtering based on
>>>>>> TraceID, available in the standard programmable replicators. e.g.,
>>>>>> Qualcomm replicators have filtering based on custom trace protocol
>>>>>> format and is not programmable.
>>>>>>
>>>>>> The source device could be connected to the replicator via
>>>>>> intermediate
>>>>>> components (e.g., a funnel). Thus we need platform information from
>>>>>> the firmware tables to decide the source device corresponding to a
>>>>>> given output port from the replicator. Given this affects "trace
>>>>>> path building" and traversing the path back from the sink to source,
>>>>>> add the concept of "filtering by source" to the generic coresight
>>>>>> connection.
>>>>>>
>>>>>> The specified source will be marked like below in the Devicetree.
>>>>>> test-replicator {
>>>>>> ... ... ... ...
>>>>>> out-ports {
>>>>>> ... ... ... ...
>>>>>> port@0 {
>>>>>> reg = <0>;
>>>>>> xyz: endpoint {
>>>>>> remote-endpoint = <&zyx>;
>>>>>> filter-source = <&source_1>; <-- To specify the
>>>>>> source to
>>>>>> }; be filtered out here.
>>>>>> };
>>>>>>
>>>>>> port@1 {
>>>>>> reg = <1>;
>>>>>> abc: endpoint {
>>>>>> remote-endpoint = <&cba>;
>>>>>> filter-source = <&source_2>; <-- To specify the
>>>>>> source to
>>>>>> }; be filtered out here.
>>>>>> };
>>>>>> };
>>>>>> };
>>>>>>
>>>>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>>>>> ---
>>>>>> drivers/hwtracing/coresight/coresight-core.c | 113
>>>>>> +++++++++++++ ++---
>>>>>> .../hwtracing/coresight/coresight-platform.c | 18 +++
>>>>>> include/linux/coresight.h | 5 +
>>>>>> 3 files changed, 117 insertions(+), 19 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/
>>>>>> drivers/hwtracing/coresight/coresight-core.c
>>>>>> index ea38ecf26fcb..0a9380350fb5 100644
>>>>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>>>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>>>>> @@ -75,22 +75,54 @@ 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 (!coresight_is_device_source(csdev))
>>>>>> + return NULL;
>>>>>> +
>>>>>> + return csdev;
>>>>>> +}
>>>>>> +
>>>>>> +/**
>>>>>> + * coresight_blocks_source - checks whether the connection
>>>>>> matches the source
>>>>>> + * of path if connection is bound to specific source.
>>>>>> + * @src: The source device of the trace path
>>>>>> + * @conn: The connection of one outport
>>>>>> + *
>>>>>> + * Return false if the connection doesn't have a source binded
>>>>>> or source of the
>>>>>> + * path matches the source binds to connection.
>>>>>> + */
>>>>>> +static bool coresight_blocks_source(struct coresight_device *src,
>>>>>> + struct coresight_connection *conn)
>>>>>> +{
>>>>>> + return conn->filter_src_fwnode && (conn->filter_src_dev !=
>>>>>> src);
>>>>>> +}
>>>>>> +
>>>>>> static struct coresight_connection *
>>>>>> -coresight_find_out_connection(struct coresight_device *src_dev,
>>>>>> - struct coresight_device *dest_dev)
>>>>>> +coresight_find_out_connection(struct coresight_device *csdev,
>>>>>> + struct coresight_device *out_dev,
>>>>>> + struct coresight_device *trace_src)
>>>>>> {
>>>>>> int i;
>>>>>> struct coresight_connection *conn;
>>>>>> - for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>>>>> - conn = src_dev->pdata->out_conns[i];
>>>>>> - if (conn->dest_dev == dest_dev)
>>>>>> + for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>>>>>> + conn = csdev->pdata->out_conns[i];
>>>>>> + if (coresight_blocks_source(trace_src, conn))
>>>>>> + continue;
>>>>>> + if (conn->dest_dev == out_dev)
>>>>>> return conn;
>>>>>> }
>>>>>> - dev_err(&src_dev->dev,
>>>>>> - "couldn't find output connection, src_dev: %s, dest_dev:
>>>>>> %s\n",
>>>>>> - dev_name(&src_dev->dev), dev_name(&dest_dev->dev));
>>>>>> + dev_err(&csdev->dev,
>>>>>> + "couldn't find output connection, csdev: %s, out_dev:
>>>>>> %s\n",
>>>>>> + dev_name(&csdev->dev), dev_name(&out_dev->dev));
>>>>>> return ERR_PTR(-ENODEV);
>>>>>> }
>>>>>> @@ -251,7 +283,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 coresight_device *source)
>>>>>> {
>>>>>> int link_subtype;
>>>>>> struct coresight_connection *inconn, *outconn;
>>>>>> @@ -259,8 +292,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, source);
>>>>>> + outconn = coresight_find_out_connection(csdev, child, source);
>>>>>> link_subtype = csdev->subtype.link_subtype;
>>>>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>>>>> IS_ERR(inconn))
>>>>>> @@ -273,15 +306,16 @@ 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 coresight_device *source)
>>>>>> {
>>>>>> struct coresight_connection *inconn, *outconn;
>>>>>> 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, source);
>>>>>> + outconn = coresight_find_out_connection(csdev, child, source);
>>>>>> link_ops(csdev)->disable(csdev, inconn, outconn);
>>>>>> }
>>>>>> @@ -375,7 +409,8 @@ 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,
>>>>>> + coresight_get_source(path));
>>>>>> break;
>>>>>> default:
>>>>>> break;
>>>>>> @@ -418,7 +453,9 @@ int coresight_enable_path(struct list_head
>>>>>> *path, enum cs_mode mode,
>>>>>> u32 type;
>>>>>> struct coresight_node *nd;
>>>>>> struct coresight_device *csdev, *parent, *child;
>>>>>> + struct coresight_device *source;
>>>>>> + source = coresight_get_source(path);
>>>>>> list_for_each_entry_reverse(nd, path, link) {
>>>>>> csdev = nd->csdev;
>>>>>> type = csdev->type;
>>>>>> @@ -456,7 +493,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,
>>>>>> source);
>>>>>> if (ret)
>>>>>> goto err;
>>>>>> break;
>>>>>> @@ -619,6 +656,7 @@ static void coresight_drop_device(struct
>>>>>> coresight_device *csdev)
>>>>>> /**
>>>>>> * _coresight_build_path - recursively build a path from a
>>>>>> @csdev to a sink.
>>>>>> * @csdev: The device to start from.
>>>>>> + * @source: The trace source device of the path.
>>>>>> * @sink: The final sink we want in this path.
>>>>>> * @path: The list to add devices to.
>>>>>> *
>>>>>> @@ -628,6 +666,7 @@ static void coresight_drop_device(struct
>>>>>> coresight_device *csdev)
>>>>>> * the source is the first device and the sink the last one.
>>>>>> */
>>>>>> static int _coresight_build_path(struct coresight_device *csdev,
>>>>>> + struct coresight_device *source,
>>>>>> struct coresight_device *sink,
>>>>>> struct list_head *path)
>>>>>> {
>>>>>> @@ -641,7 +680,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, source, sink, path) == 0) {
>>>>>> found = true;
>>>>>> goto out;
>>>>>> }
>>>>>> @@ -652,8 +691,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 (coresight_blocks_source(source, csdev->pdata-
>>>>>> >out_conns[i]))
>>>>>> + continue;
>>>>>> +
>>>>>> if (child_dev &&
>>>>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>>>>> + _coresight_build_path(child_dev, source, sink, path)
>>>>>> == 0) {
>>>>>> found = true;
>>>>>> break;
>>>>>> }
>>>>>> @@ -698,7 +741,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, source, sink, path);
>>>>>> if (rc) {
>>>>>> kfree(path);
>>>>>> return ERR_PTR(rc);
>>>>>> @@ -927,6 +970,16 @@ static int coresight_orphan_match(struct
>>>>>> device *dev, void *data)
>>>>>> for (i = 0; i < src_csdev->pdata->nr_outconns; i++) {
>>>>>> conn = src_csdev->pdata->out_conns[i];
>>>>>> + /* Fix filter source device before skip the port */
>>>>>> + if (conn->filter_src_fwnode && !conn->filter_src_dev) {
>>>>>> + if (dst_csdev &&
>>>>>> + (conn->filter_src_fwnode ==
>>>>>> dst_csdev->dev.fwnode) &&
>>>>>> + !WARN_ON_ONCE(!coresight_is_device_source(dst_csdev)))
>>>>>> + conn->filter_src_dev = dst_csdev;
>>>>>> + else
>>>>>> + still_orphan = true;
>>>>>> + }
>>>>>> +
>>>>>> /* Skip the port if it's already connected. */
>>>>>> if (conn->dest_dev)
>>>>>> continue;
>>>>>> @@ -977,18 +1030,40 @@ static int
>>>>>> coresight_fixup_orphan_conns(struct coresight_device *csdev)
>>>>>> csdev, coresight_orphan_match);
>>>>>> }
>>>>>> +static int coresight_clear_filter_source(struct device *dev,
>>>>>> void *data)
>>>>>> +{
>>>>>> + int i;
>>>>>> + struct coresight_device *source = data;
>>>>>> + struct coresight_device *csdev = to_coresight_device(dev);
>>>>>> +
>>>>>> + for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
>>>>>> + if (csdev->pdata->out_conns[i]->filter_src_dev == source)
>>>>>> + csdev->pdata->out_conns[i]->filter_src_dev = NULL;
>>>>>> + }
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> /* coresight_remove_conns - Remove other device's references to
>>>>>> this device */
>>>>>> static void coresight_remove_conns(struct coresight_device *csdev)
>>>>>> {
>>>>>> int i, j;
>>>>>> struct coresight_connection *conn;
>>>>>> + if (coresight_is_device_source(csdev))
>>>>>> + bus_for_each_dev(&coresight_bustype, NULL, csdev,
>>>>>> + coresight_clear_filter_source);
>>>>>> +
>>>>>> /*
>>>>>> * Remove the input connection references from the
>>>>>> destination device
>>>>>> * for each output connection.
>>>>>> */
>>>>>> for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>>>>>> conn = csdev->pdata->out_conns[i];
>>>>>> + if (conn->filter_src_fwnode) {
>>>>>> + conn->filter_src_dev = NULL;
>>>>>> + fwnode_handle_put(conn->filter_src_fwnode);
>>>>>> + }
>>>>>> +
>>>>>> if (!conn->dest_dev)
>>>>>> continue;
>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-platform.c
>>>>>> b/ drivers/hwtracing/coresight/coresight-platform.c
>>>>>> index 64e171eaad82..d5532caa9e92 100644
>>>>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>>>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>>>>> @@ -243,6 +243,24 @@ static int
>>>>>> of_coresight_parse_endpoint(struct device *dev,
>>>>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>>>>> conn.dest_port = rendpoint.port;
>>>>>> + /*
>>>>>> + * Get the firmware node of the filter source through the
>>>>>> + * reference. This could be used to filter the source in
>>>>>> + * building path.
>>>>>> + */
>>>>>> + conn.filter_src_fwnode =
>>>>>> + fwnode_find_reference(&ep->fwnode, "filter-source", 0);
>>>>>> + if (IS_ERR(conn.filter_src_fwnode)) {
>>>>>> + conn.filter_src_fwnode = NULL;
>>>>>> + } else {
>>>>>> + conn.filter_src_dev =
>>>>>> + coresight_find_csdev_by_fwnode(conn.filter_src_fwnode);
>>>>>> + if (conn.filter_src_dev &&
>>>>>> + !coresight_is_device_source(conn.filter_src_dev))
>>>>>> + dev_warn(&conn.filter_src_dev->dev,
>>>>>> + "Filter source is not a source device\n");
>>>>>
>>>>> Do we need to reset the conn.filter_src_fwnode ? Otherwise, we can
>>>>> warn from other places too.
>>>>
>>>> Agree, we need to reset conn.filter_src_fwnode and
>>>> conn.filter_src_dev here if it is not a SOURCE
>>>>
>>>> type device.
>>>>
>>>> I will update the following code to the next patch series. Could
>>>> you help check if it is fine to you?
>>>
>>> This looks good.
>>>
>>>>
>>>> + if (conn.filter_src_dev &&
>>>> + !coresight_is_device_source(conn.filter_src_dev)) {
>>>> + dev_warn(&conn.filter_src_dev->dev,
>>>> + "Filter source is not a source device\n");
>>>
>>> minor nit: It would be good to indicate which device has this
>>> "filtering". i.e,
>>> dev_warn(dev, "Filter source is not a trace source :
>>> %s\n", dev_name(&conn.filter_src_dev->dev));
>> Sure, I will update this to the next patch series.
>
> Also, it may be helpful, if we specify the "port number" which has this
> wrong handle. I guess something like:
>
> dev_warn(dev, "port %d: Filter handle is not a trace source : %s\n",
> conn.src_port, dev_name(&conn.filter_src_dev->dev));
Sure, I will update this to the next patch series.
Best,
Tao
>
> Suzuki
>
>>>
>>>> + conn.filter_src_dev = NULL；
>>>> + conn.filter_src_dev = NULL;
>>
>> Sorry for my mistake here. I also want to set "filter_src_fwnode " to
>> NULL if it is not a trace source.
>>
>> conn.filter_src_fwnode = NULL;
>
>
>>
>>
>> Best,
>>
>> Tao
>>
>>>
>>> Duplicated line ^
>>>
>>> Suzuki
>>>
>>>> + }
>>>>
>>>> Best,
>>>>
>>>> Tao
>>>>
>>>>>
>>>>> Suzuki
>>>>>
>>>>>> + }
>>>>>> +
>>>>>> 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 9311df8538fc..f372c01ae2fc 100644
>>>>>> --- a/include/linux/coresight.h
>>>>>> +++ b/include/linux/coresight.h
>>>>>> @@ -172,6 +172,9 @@ struct coresight_desc {
>>>>>> * @dest_dev: a @coresight_device representation of the
>>>>>> component
>>>>>> connected to @src_port. NULL until the device is created
>>>>>> * @link: Representation of the connection as a sysfs link.
>>>>>> + * @filter_src_fwnode: filter source component's fwnode handle.
>>>>>> + * @filter_src_dev: a @coresight_device representation of the
>>>>>> component that
>>>>>> + needs to be filtered.
>>>>>> *
>>>>>> * The full connection structure looks like this, where
>>>>>> in_conns store
>>>>>> * references to same connection as the source device's out_conns.
>>>>>> @@ -200,6 +203,8 @@ struct coresight_connection {
>>>>>> struct coresight_device *dest_dev;
>>>>>> struct coresight_sysfs_link *link;
>>>>>> struct coresight_device *src_dev;
>>>>>> + struct fwnode_handle *filter_src_fwnode;
>>>>>> + struct coresight_device *filter_src_dev;
>>>>>> atomic_t src_refcnt;
>>>>>> atomic_t dest_refcnt;
>>>>>> };
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-11-19 8:59 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-30 9:32 [PATCH v5 0/4] source filtering for multi-port output Tao Zhang
2024-10-30 9:32 ` [PATCH v5 1/4] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering Tao Zhang
2024-10-30 9:32 ` [PATCH v5 2/4] coresight: Add a helper to check if a device is source Tao Zhang
2024-10-30 9:32 ` [PATCH v5 3/4] coresight: Add support for trace filtering by source Tao Zhang
2024-11-13 13:39 ` Suzuki K Poulose
2024-11-15 9:18 ` Tao Zhang
2024-11-15 9:40 ` Suzuki K Poulose
2024-11-18 9:28 ` Tao Zhang
2024-11-18 9:56 ` Suzuki K Poulose
2024-11-19 8:58 ` Tao Zhang
2024-10-30 9:32 ` [PATCH v5 4/4] coresight-tpda: Optimize the function of reading element size Tao Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox