[PATCH v1 0/3] Add source filtering for multi-port output

[PATCH v1 0/3] Add source filtering for multi-port output

[Tao Zhang]

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 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 (3):
  dt-bindings: arm: qcom,coresight-static-replicator: Add property for
    source filtering
  coresight: Add source filtering for multi-port output
  coresight-tpda: Optimize the function of reading element size

 .../arm/arm,coresight-static-replicator.yaml  | 18 +++-
 drivers/hwtracing/coresight/coresight-core.c  | 89 ++++++++++++++++---
 .../hwtracing/coresight/coresight-platform.c  | 13 +++
 drivers/hwtracing/coresight/coresight-tpda.c  |  6 +-
 include/linux/coresight.h                     |  5 ++
 5 files changed, 115 insertions(+), 16 deletions(-)

-- 
2.17.1

[PATCH v1 1/3] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering

[Tao Zhang]

Add a new property "filter_src" to label the source corresponding
to the output connection for a static replicator. By combining
a funnel and a static replicator in devicetree, a new device that
supports multi-port input and multi-port output is implemented.
In order to match the output port with the input port and
successfully build the trace path, add this new property to
indicate the data source corresponding to this output port.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 .../arm/arm,coresight-static-replicator.yaml   | 18 +++++++++++++++++-
 1 file changed, 17 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..d9538563f9c6 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
@@ -45,7 +45,21 @@ properties:
     patternProperties:
       '^port@[01]$':
         description: Output connections to CoreSight Trace bus
-        $ref: /schemas/graph.yaml#/properties/port
+        $ref: /schemas/graph.yaml#/$defs/port-base
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+
+            properties:
+              filter_src:
+                $ref: /schemas/types.yaml#/definitions/phandle
+                description:
+                  defines a phandle reference to an associated CoreSight trace device.
+                  When the associated trace device is enabled, then the respective
+                  trace path will be built and enabled.
+
+              remote-endpoint: true
 
 required:
   - compatible
@@ -72,6 +86,7 @@ examples:
                 reg = <0>;
                 replicator_out_port0: endpoint {
                     remote-endpoint = <&etb_in_port>;
+                    filter_src = <&tpdm_video>;
                 };
             };
 
@@ -79,6 +94,7 @@ examples:
                 reg = <1>;
                 replicator_out_port1: endpoint {
                     remote-endpoint = <&tpiu_in_port>;
+                    filter_src = <&tpdm_mdss>;
                 };
             };
         };
-- 
2.17.1

Re: [PATCH v1 1/3] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering

[Krzysztof Kozlowski]

On 05/07/2024 10:51, Tao Zhang wrote:
> Add a new property "filter_src" to label the source corresponding
> to the output connection for a static replicator. By combining
> a funnel and a static replicator in devicetree, a new device that
> supports multi-port input and multi-port output is implemented.
> In order to match the output port with the input port and
> successfully build the trace path, add this new property to
> indicate the data source corresponding to this output port.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>  .../arm/arm,coresight-static-replicator.yaml   | 18 +++++++++++++++++-
>  1 file changed, 17 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..d9538563f9c6 100644
> --- a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
> +++ b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
> @@ -45,7 +45,21 @@ properties:
>      patternProperties:
>        '^port@[01]$':
>          description: Output connections to CoreSight Trace bus
> -        $ref: /schemas/graph.yaml#/properties/port
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#

Ehm? How is this video interface?

> +
> +            properties:
> +              filter_src:

There are no properties with underscores...

> +                $ref: /schemas/types.yaml#/definitions/phandle
> +                description:
> +                  defines a phandle reference to an associated CoreSight trace device.
> +                  When the associated trace device is enabled, then the respective
> +                  trace path will be built and enabled.

How does it differ from remote endpoint? What is "respective trace path"?

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.
</form letter>


Best regards,
Krzysztof

Re: [PATCH v1 1/3] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering

[Suzuki K Poulose]

minor nit: Subject: 
s/qcom,coresight-static-replicator/arm,coresight-static-replicator ? 
There is no "qcom,coresight-static-replicator" compatible.

On 05/07/2024 10:02, Krzysztof Kozlowski wrote:

> On 05/07/2024 10:51, Tao Zhang wrote:
>> Add a new property "filter_src" to label the source corresponding
>> to the output connection for a static replicator. By combining
>> a funnel and a static replicator in devicetree, a new device that
>> supports multi-port input and multi-port output is implemented.
>> In order to match the output port with the input port and
>> successfully build the trace path, add this new property to
>> indicate the data source corresponding to this output port.
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> ---
>>   .../arm/arm,coresight-static-replicator.yaml   | 18 +++++++++++++++++-
>>   1 file changed, 17 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..d9538563f9c6 100644
>> --- a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
>> +++ b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
>> @@ -45,7 +45,21 @@ properties:
>>       patternProperties:
>>         '^port@[01]$':
>>           description: Output connections to CoreSight Trace bus
>> -        $ref: /schemas/graph.yaml#/properties/port
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +
>> +        properties:
>> +          endpoint:
>> +            $ref: /schemas/media/video-interfaces.yaml#
> 
> Ehm? How is this video interface?
> 
>> +
>> +            properties:
>> +              filter_src:
> 
> There are no properties with underscores...
> 
>> +                $ref: /schemas/types.yaml#/definitions/phandle
>> +                description:
>> +                  defines a phandle reference to an associated CoreSight trace device.
>> +                  When the associated trace device is enabled, then the respective
>> +                  trace path will be built and enabled.
> 
> How does it differ from remote endpoint? What is "respective trace path"?

Apparently, there is some "magic" hard coded filtering in the
replicators, which only passes through trace from a particular "source"
device. The documentation above doesn't explain this clearly.

it could be:

"phandle to the coresight trace source device matching the hard coded
filtering for this port"

This could be different from the "remote endpoint" as there could be
intermediate components between the phandle "source" and the port.


Suzuki



> 
> <form letter>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC (and consider --no-git-fallback argument). It might
> happen, that command when run on an older kernel, gives you outdated
> entries. Therefore please be sure you base your patches on recent Linux
> kernel.
> 
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline) or work on fork of kernel
> (don't, instead use mainline). Just use b4 and everything should be
> fine, although remember about `b4 prep --auto-to-cc` if you added new
> patches to the patchset.
> </form letter>
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v1 1/3] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering

[Rob Herring (Arm)]

On Fri, 05 Jul 2024 16:51:50 +0800, Tao Zhang wrote:
> Add a new property "filter_src" to label the source corresponding
> to the output connection for a static replicator. By combining
> a funnel and a static replicator in devicetree, a new device that
> supports multi-port input and multi-port output is implemented.
> In order to match the output port with the input port and
> successfully build the trace path, add this new property to
> indicate the data source corresponding to this output port.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>  .../arm/arm,coresight-static-replicator.yaml   | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml: ^port@[01]$: Missing additionalProperties/unevaluatedProperties constraint
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml: endpoint: Missing additionalProperties/unevaluatedProperties constraint

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240705085152.9063-2-quic_taozha@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v1 1/3] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering

[Tao Zhang]

On 7/5/2024 6:38 PM, Rob Herring (Arm) wrote:
> On Fri, 05 Jul 2024 16:51:50 +0800, Tao Zhang wrote:
>> Add a new property "filter_src" to label the source corresponding
>> to the output connection for a static replicator. By combining
>> a funnel and a static replicator in devicetree, a new device that
>> supports multi-port input and multi-port output is implemented.
>> In order to match the output port with the input port and
>> successfully build the trace path, add this new property to
>> indicate the data source corresponding to this output port.
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> ---
>>   .../arm/arm,coresight-static-replicator.yaml   | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml: ^port@[01]$: Missing additionalProperties/unevaluatedProperties constraint
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml: endpoint: Missing additionalProperties/unevaluatedProperties constraint
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240705085152.9063-2-quic_taozha@quicinc.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade

Yes, I didn't see this errors in running 'make dt_binding_check', I will 
re-run this check

according to your suggestion.


Best,

Tao

>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>

Re: [PATCH v1 1/3] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering

[Tao Zhang]

On 7/5/2024 11:23 PM, Tao Zhang wrote:
>
> On 7/5/2024 6:38 PM, Rob Herring (Arm) wrote:
>> On Fri, 05 Jul 2024 16:51:50 +0800, Tao Zhang wrote:
>>> Add a new property "filter_src" to label the source corresponding
>>> to the output connection for a static replicator. By combining
>>> a funnel and a static replicator in devicetree, a new device that
>>> supports multi-port input and multi-port output is implemented.
>>> In order to match the output port with the input port and
>>> successfully build the trace path, add this new property to
>>> indicate the data source corresponding to this output port.
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>> ---
>>>   .../arm/arm,coresight-static-replicator.yaml   | 18 
>>> +++++++++++++++++-
>>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml: 
>> ^port@[01]$: Missing additionalProperties/unevaluatedProperties 
>> constraint
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml: 
>> endpoint: Missing additionalProperties/unevaluatedProperties constraint
>>
>> doc reference errors (make refcheckdocs):
>>
>> See 
>> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240705085152.9063-2-quic_taozha@quicinc.com
>>
>> The base for the series is generally the latest rc1. A different 
>> dependency
>> should be noted in *this* patch.
>>
>> If you already ran 'make dt_binding_check' and didn't see the above
>> error(s), then make sure 'yamllint' is installed and dt-schema is up to
>> date:
>>
>> pip3 install dtschema --upgrade
>
> Yes, I didn't see this errors in running 'make dt_binding_check', I 
> will re-run this check
>
> according to your suggestion.
>
>
> Best,
>
> Tao

After upgrading dtschema and installing 'yamllint', I saw the above 
errors in running

'dt_binding_check'. I will fix them and update in the next patch series.


Best,

Tao

>
>>
>> Please check and re-submit after running the above command yourself. 
>> Note
>> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
>> your schema. However, it must be unset to test all examples with your 
>> schema.
>>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org

[PATCH v1 2/3] coresight: Add source filtering for multi-port output

[Tao Zhang]

In order to enable the output ports of multi-port output devices,
such as static replicator, to correspond to designated sources,
a mechanism for filtering data sources is introduced for the
output ports.

The specified source will be marked like below in the Devicetree.
test-replicator {
    ... ... ... ...
    out-ports {
        ... ... ... ...
        port@0 {
            reg = <0>;
            xxx: endpoint {
                remote-endpoint = <&xxx>;
                filter_src = <&xxx>; <-- To specify the source to
            };                           be filtered out here.
        };

        port@1 {
            reg = <1>;
            yyy: endpoint {
                remote-endpoint = <&yyy>;
                filter_src = <&yyy>; <-- To specify the source to
            };                           be filtered out here.
        };
    };
};

Then driver will find the expected source marked in the Devicetree, and
save it to the coresight path. When the function needs to filter the
source, it could obtain it from coresight path parameter. Finally,
the output port knows which source it corresponds to, and it also knows
which input port it corresponds to.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 drivers/hwtracing/coresight/coresight-core.c  | 89 ++++++++++++++++---
 .../hwtracing/coresight/coresight-platform.c  | 13 +++
 include/linux/coresight.h                     |  5 ++
 3 files changed, 94 insertions(+), 13 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 5dde597403b3..1c58b64f0031 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -113,15 +113,62 @@ struct coresight_device *coresight_get_percpu_sink(int cpu)
 }
 EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
 
+static struct coresight_device *coresight_get_source(struct list_head *path)
+{
+	struct coresight_device *csdev;
+
+	if (!path)
+		return NULL;
+
+	csdev = list_first_entry(path, struct coresight_node, link)->csdev;
+	if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
+		return NULL;
+
+	return csdev;
+}
+
+/**
+ * coresight_source_filter - checks whether the connection matches the source
+ * of path if connection is binded to specific source.
+ * @path:	The list of devices
+ * @conn:	The connection of one outport
+ *
+ * Return zero if the connection doesn't have a source binded or source of the
+ * path matches the source binds to connection.
+ */
+static int coresight_source_filter(struct list_head *path,
+			struct coresight_connection *conn)
+{
+	int ret = 0;
+	struct coresight_device *source = NULL;
+
+	if (!conn->filter_src_dev)
+		return ret;
+
+	source = coresight_get_source(path);
+	if (!source)
+		return ret;
+
+	if (conn->filter_src_dev == source)
+		ret = 0;
+	else
+		ret = -1;
+
+	return ret;
+}
+
 static struct coresight_connection *
 coresight_find_out_connection(struct coresight_device *src_dev,
-			      struct coresight_device *dest_dev)
+			      struct coresight_device *dest_dev,
+			      struct list_head *path)
 {
 	int i;
 	struct coresight_connection *conn;
 
 	for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
 		conn = src_dev->pdata->out_conns[i];
+		if (coresight_source_filter(path, conn))
+			continue;
 		if (conn->dest_dev == dest_dev)
 			return conn;
 	}
@@ -312,7 +359,8 @@ static void coresight_disable_sink(struct coresight_device *csdev)
 
 static int coresight_enable_link(struct coresight_device *csdev,
 				 struct coresight_device *parent,
-				 struct coresight_device *child)
+				 struct coresight_device *child,
+				 struct list_head *path)
 {
 	int ret = 0;
 	int link_subtype;
@@ -321,8 +369,8 @@ static int coresight_enable_link(struct coresight_device *csdev,
 	if (!parent || !child)
 		return -EINVAL;
 
-	inconn = coresight_find_out_connection(parent, csdev);
-	outconn = coresight_find_out_connection(csdev, child);
+	inconn = coresight_find_out_connection(parent, csdev, path);
+	outconn = coresight_find_out_connection(csdev, child, path);
 	link_subtype = csdev->subtype.link_subtype;
 
 	if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && IS_ERR(inconn))
@@ -341,7 +389,8 @@ static int coresight_enable_link(struct coresight_device *csdev,
 
 static void coresight_disable_link(struct coresight_device *csdev,
 				   struct coresight_device *parent,
-				   struct coresight_device *child)
+				   struct coresight_device *child,
+				   struct list_head *path)
 {
 	int i;
 	int link_subtype;
@@ -350,8 +399,8 @@ static void coresight_disable_link(struct coresight_device *csdev,
 	if (!parent || !child)
 		return;
 
-	inconn = coresight_find_out_connection(parent, csdev);
-	outconn = coresight_find_out_connection(csdev, child);
+	inconn = coresight_find_out_connection(parent, csdev, path);
+	outconn = coresight_find_out_connection(csdev, child, path);
 	link_subtype = csdev->subtype.link_subtype;
 
 	if (link_ops(csdev)->disable) {
@@ -507,7 +556,7 @@ static void coresight_disable_path_from(struct list_head *path,
 		case CORESIGHT_DEV_TYPE_LINK:
 			parent = list_prev_entry(nd, link)->csdev;
 			child = list_next_entry(nd, link)->csdev;
-			coresight_disable_link(csdev, parent, child);
+			coresight_disable_link(csdev, parent, child, path);
 			break;
 		default:
 			break;
@@ -588,7 +637,7 @@ int coresight_enable_path(struct list_head *path, enum cs_mode mode,
 		case CORESIGHT_DEV_TYPE_LINK:
 			parent = list_prev_entry(nd, link)->csdev;
 			child = list_next_entry(nd, link)->csdev;
-			ret = coresight_enable_link(csdev, parent, child);
+			ret = coresight_enable_link(csdev, parent, child, path);
 			if (ret)
 				goto err;
 			break;
@@ -802,7 +851,8 @@ static void coresight_drop_device(struct coresight_device *csdev)
  */
 static int _coresight_build_path(struct coresight_device *csdev,
 				 struct coresight_device *sink,
-				 struct list_head *path)
+				 struct list_head *path,
+				 struct coresight_device *trace_source)
 {
 	int i, ret;
 	bool found = false;
@@ -814,7 +864,7 @@ static int _coresight_build_path(struct coresight_device *csdev,
 
 	if (coresight_is_percpu_source(csdev) && coresight_is_percpu_sink(sink) &&
 	    sink == per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev))) {
-		if (_coresight_build_path(sink, sink, path) == 0) {
+		if (_coresight_build_path(sink, sink, path, trace_source) == 0) {
 			found = true;
 			goto out;
 		}
@@ -825,8 +875,13 @@ static int _coresight_build_path(struct coresight_device *csdev,
 		struct coresight_device *child_dev;
 
 		child_dev = csdev->pdata->out_conns[i]->dest_dev;
+
+		if (csdev->pdata->out_conns[i]->filter_src_dev
+		    && (csdev->pdata->out_conns[i]->filter_src_dev != trace_source))
+			continue;
+
 		if (child_dev &&
-		    _coresight_build_path(child_dev, sink, path) == 0) {
+		    _coresight_build_path(child_dev, sink, path, trace_source) == 0) {
 			found = true;
 			break;
 		}
@@ -871,7 +926,7 @@ struct list_head *coresight_build_path(struct coresight_device *source,
 
 	INIT_LIST_HEAD(path);
 
-	rc = _coresight_build_path(source, sink, path);
+	rc = _coresight_build_path(source, sink, path, source);
 	if (rc) {
 		kfree(path);
 		return ERR_PTR(rc);
@@ -1395,6 +1450,9 @@ static int coresight_orphan_match(struct device *dev, void *data)
 			/* This component still has an orphan */
 			still_orphan = true;
 		}
+		if ((conn->filter_src_fwnode) && dst_csdev
+		    && (conn->filter_src_fwnode == dst_csdev->dev.fwnode))
+			conn->filter_src_dev = dst_csdev;
 	}
 
 	src_csdev->orphan = still_orphan;
@@ -1424,6 +1482,11 @@ static void coresight_remove_conns(struct coresight_device *csdev)
 	 */
 	for (i = 0; i < csdev->pdata->nr_outconns; i++) {
 		conn = csdev->pdata->out_conns[i];
+		if (conn->filter_src_dev) {
+			conn->filter_src_dev = NULL;
+			conn->filter_src_fwnode = NULL;
+		}
+
 		if (!conn->dest_dev)
 			continue;
 
diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
index 9d550f5697fa..a9f5b0700310 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -243,6 +243,19 @@ 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_src", 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);
+
 		new_conn = coresight_add_out_conn(dev, pdata, &conn);
 		if (IS_ERR_VALUE(new_conn)) {
 			fwnode_handle_put(conn.dest_fwnode);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index e8b6e388218c..0a7ec0978605 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -171,6 +171,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.
@@ -199,6 +202,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

Re: [PATCH v1 2/3] coresight: Add source filtering for multi-port output

[Suzuki K Poulose]

On 05/07/2024 09:51, Tao Zhang wrote:
> In order to enable the output ports of multi-port output devices,
> such as static replicator, to correspond to designated sources,
> a mechanism for filtering data sources is introduced for the
> output ports.
> 
> The specified source will be marked like below in the Devicetree.
> test-replicator {
>      ... ... ... ...
>      out-ports {
>          ... ... ... ...
>          port@0 {
>              reg = <0>;
>              xxx: endpoint {
>                  remote-endpoint = <&xxx>;
>                  filter_src = <&xxx>; <-- To specify the source to
>              };                           be filtered out here.
>          };
> 
>          port@1 {
>              reg = <1>;
>              yyy: endpoint {
>                  remote-endpoint = <&yyy>;
>                  filter_src = <&yyy>; <-- To specify the source to
>              };                           be filtered out here.
>          };
>      };
> };
> 
> Then driver will find the expected source marked in the Devicetree, and
> save it to the coresight path. When the function needs to filter the
> source, it could obtain it from coresight path parameter. Finally,
> the output port knows which source it corresponds to, and it also knows
> which input port it corresponds to.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c  | 89 ++++++++++++++++---
>   .../hwtracing/coresight/coresight-platform.c  | 13 +++
>   include/linux/coresight.h                     |  5 ++
>   3 files changed, 94 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 5dde597403b3..1c58b64f0031 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -113,15 +113,62 @@ struct coresight_device *coresight_get_percpu_sink(int cpu)
>   }
>   EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>   
> +static struct coresight_device *coresight_get_source(struct list_head *path)
> +{
> +	struct coresight_device *csdev;
> +
> +	if (!path)
> +		return NULL;
> +
> +	csdev = list_first_entry(path, struct coresight_node, link)->csdev;
> +	if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
> +		return NULL;
> +
> +	return csdev;
> +}
> +
> +/**
> + * coresight_source_filter - checks whether the connection matches the source
> + * of path if connection is binded to specific source.
> + * @path:	The list of devices
> + * @conn:	The connection of one outport
> + *
> + * Return zero if the connection doesn't have a source binded or source of the
> + * path matches the source binds to connection.
> + */
> +static int coresight_source_filter(struct list_head *path,

Instead of path, you may need to switch to "source" device. See below.

> +			struct coresight_connection *conn)
> +{
> +	int ret = 0;
> +	struct coresight_device *source = NULL;
> +
> +	if (!conn->filter_src_dev)
> +		return ret;
> +
> +	source = coresight_get_source(path);
> +	if (!source)
> +		return ret;
> +
> +	if (conn->filter_src_dev == source)
> +		ret = 0;
> +	else
> +		ret = -1;
> +
> +	return ret;
> +}
> +
>   static struct coresight_connection *
>   coresight_find_out_connection(struct coresight_device *src_dev,
> -			      struct coresight_device *dest_dev)
> +			      struct coresight_device *dest_dev,
> +			      struct list_head *path)
>   {
>   	int i;
>   	struct coresight_connection *conn;
>   
>   	for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>   		conn = src_dev->pdata->out_conns[i];
> +		if (coresight_source_filter(path, conn))
> +			continue;
>   		if (conn->dest_dev == dest_dev)
>   			return conn;
>   	}
> @@ -312,7 +359,8 @@ static void coresight_disable_sink(struct coresight_device *csdev)
>   
>   static int coresight_enable_link(struct coresight_device *csdev,
>   				 struct coresight_device *parent,
> -				 struct coresight_device *child)
> +				 struct coresight_device *child,
> +				 struct list_head *path)
>   {
>   	int ret = 0;
>   	int link_subtype;
> @@ -321,8 +369,8 @@ static int coresight_enable_link(struct coresight_device *csdev,
>   	if (!parent || !child)
>   		return -EINVAL;
>   
> -	inconn = coresight_find_out_connection(parent, csdev);
> -	outconn = coresight_find_out_connection(csdev, child);
> +	inconn = coresight_find_out_connection(parent, csdev, path);
> +	outconn = coresight_find_out_connection(csdev, child, path);
>   	link_subtype = csdev->subtype.link_subtype;
>   
>   	if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && IS_ERR(inconn))
> @@ -341,7 +389,8 @@ static int coresight_enable_link(struct coresight_device *csdev,
>   
>   static void coresight_disable_link(struct coresight_device *csdev,
>   				   struct coresight_device *parent,
> -				   struct coresight_device *child)
> +				   struct coresight_device *child,
> +				   struct list_head *path)
>   {
>   	int i;
>   	int link_subtype;
> @@ -350,8 +399,8 @@ static void coresight_disable_link(struct coresight_device *csdev,
>   	if (!parent || !child)
>   		return;
>   
> -	inconn = coresight_find_out_connection(parent, csdev);
> -	outconn = coresight_find_out_connection(csdev, child);
> +	inconn = coresight_find_out_connection(parent, csdev, path);
> +	outconn = coresight_find_out_connection(csdev, child, path);
>   	link_subtype = csdev->subtype.link_subtype;
>   
>   	if (link_ops(csdev)->disable) {
> @@ -507,7 +556,7 @@ static void coresight_disable_path_from(struct list_head *path,
>   		case CORESIGHT_DEV_TYPE_LINK:
>   			parent = list_prev_entry(nd, link)->csdev;
>   			child = list_next_entry(nd, link)->csdev;
> -			coresight_disable_link(csdev, parent, child);
> +			coresight_disable_link(csdev, parent, child, path);
>   			break;

Disable path from could be called with a "partial path" with the
"source" stripped off. e.g, if the enabling of the components failed
mid-point in the path. So, source_from_path() above is WRONG csdev.
Instead you should make sure we pass the "source", which must be
available with the caller.


>   		default:
>   			break;
> @@ -588,7 +637,7 @@ int coresight_enable_path(struct list_head *path, enum cs_mode mode,
>   		case CORESIGHT_DEV_TYPE_LINK:
>   			parent = list_prev_entry(nd, link)->csdev;
>   			child = list_next_entry(nd, link)->csdev;
> -			ret = coresight_enable_link(csdev, parent, child);
> +			ret = coresight_enable_link(csdev, parent, child, path);
>   			if (ret)
>   				goto err;
>   			break;
> @@ -802,7 +851,8 @@ static void coresight_drop_device(struct coresight_device *csdev)
>    */
>   static int _coresight_build_path(struct coresight_device *csdev,
>   				 struct coresight_device *sink,
> -				 struct list_head *path)
> +				 struct list_head *path,
> +				 struct coresight_device *trace_source)
>   {
>   	int i, ret;
>   	bool found = false;
> @@ -814,7 +864,7 @@ static int _coresight_build_path(struct coresight_device *csdev,
>   
>   	if (coresight_is_percpu_source(csdev) && coresight_is_percpu_sink(sink) &&
>   	    sink == per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev))) {
> -		if (_coresight_build_path(sink, sink, path) == 0) {
> +		if (_coresight_build_path(sink, sink, path, trace_source) == 0) {
>   			found = true;
>   			goto out;
>   		}
> @@ -825,8 +875,13 @@ static int _coresight_build_path(struct coresight_device *csdev,
>   		struct coresight_device *child_dev;
>   
>   		child_dev = csdev->pdata->out_conns[i]->dest_dev;
> +
> +		if (csdev->pdata->out_conns[i]->filter_src_dev
> +		    && (csdev->pdata->out_conns[i]->filter_src_dev != trace_source))
> +			continue;
> +
>   		if (child_dev &&
> -		    _coresight_build_path(child_dev, sink, path) == 0) {
> +		    _coresight_build_path(child_dev, sink, path, trace_source) == 0) {
>   			found = true;
>   			break;
>   		}
> @@ -871,7 +926,7 @@ struct list_head *coresight_build_path(struct coresight_device *source,
>   
>   	INIT_LIST_HEAD(path);
>   
> -	rc = _coresight_build_path(source, sink, path);
> +	rc = _coresight_build_path(source, sink, path, source);
>   	if (rc) {
>   		kfree(path);
>   		return ERR_PTR(rc);
> @@ -1395,6 +1450,9 @@ static int coresight_orphan_match(struct device *dev, void *data)
>   			/* This component still has an orphan */
>   			still_orphan = true;
>   		}
> +		if ((conn->filter_src_fwnode) && dst_csdev
> +		    && (conn->filter_src_fwnode == dst_csdev->dev.fwnode))
> +			conn->filter_src_dev = dst_csdev;
>   	}
>   
>   	src_csdev->orphan = still_orphan;
> @@ -1424,6 +1482,11 @@ static void coresight_remove_conns(struct coresight_device *csdev)
>   	 */
>   	for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>   		conn = csdev->pdata->out_conns[i];
> +		if (conn->filter_src_dev) {
> +			conn->filter_src_dev = NULL;
> +			conn->filter_src_fwnode = NULL;
> +		}
> +

Similarly we should reset the "filter_src_dev" if the "src" csdev is
being removed.  You may need a new function for that, which scans
through all devices and looks for a conn->filter_src_dev == csdev.
Something like:



>   		if (!conn->dest_dev)
>   			continue;
>   
> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
> index 9d550f5697fa..a9f5b0700310 100644
> --- a/drivers/hwtracing/coresight/coresight-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> @@ -243,6 +243,19 @@ 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_src", 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);

We should warn, if the filter_src_dev is of not the type DEV_TYPE_SOURCE.

> +
>   		new_conn = coresight_add_out_conn(dev, pdata, &conn);
>   		if (IS_ERR_VALUE(new_conn)) {
>   			fwnode_handle_put(conn.dest_fwnode);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index e8b6e388218c..0a7ec0978605 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -171,6 +171,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.
> @@ -199,6 +202,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;
>   };

Suzuki

Re: [PATCH v1 2/3] coresight: Add source filtering for multi-port output

[Tao Zhang]

On 7/9/2024 9:42 PM, Suzuki K Poulose wrote:
> On 05/07/2024 09:51, Tao Zhang wrote:
>> In order to enable the output ports of multi-port output devices,
>> such as static replicator, to correspond to designated sources,
>> a mechanism for filtering data sources is introduced for the
>> output ports.
>>
>> The specified source will be marked like below in the Devicetree.
>> test-replicator {
>>      ... ... ... ...
>>      out-ports {
>>          ... ... ... ...
>>          port@0 {
>>              reg = <0>;
>>              xxx: endpoint {
>>                  remote-endpoint = <&xxx>;
>>                  filter_src = <&xxx>; <-- To specify the source to
>>              };                           be filtered out here.
>>          };
>>
>>          port@1 {
>>              reg = <1>;
>>              yyy: endpoint {
>>                  remote-endpoint = <&yyy>;
>>                  filter_src = <&yyy>; <-- To specify the source to
>>              };                           be filtered out here.
>>          };
>>      };
>> };
>>
>> Then driver will find the expected source marked in the Devicetree, and
>> save it to the coresight path. When the function needs to filter the
>> source, it could obtain it from coresight path parameter. Finally,
>> the output port knows which source it corresponds to, and it also knows
>> which input port it corresponds to.
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-core.c  | 89 ++++++++++++++++---
>>   .../hwtracing/coresight/coresight-platform.c  | 13 +++
>>   include/linux/coresight.h                     |  5 ++
>>   3 files changed, 94 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c 
>> b/drivers/hwtracing/coresight/coresight-core.c
>> index 5dde597403b3..1c58b64f0031 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -113,15 +113,62 @@ struct coresight_device 
>> *coresight_get_percpu_sink(int cpu)
>>   }
>>   EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>   +static struct coresight_device *coresight_get_source(struct 
>> list_head *path)
>> +{
>> +    struct coresight_device *csdev;
>> +
>> +    if (!path)
>> +        return NULL;
>> +
>> +    csdev = list_first_entry(path, struct coresight_node, link)->csdev;
>> +    if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
>> +        return NULL;
>> +
>> +    return csdev;
>> +}
>> +
>> +/**
>> + * coresight_source_filter - checks whether the connection matches 
>> the source
>> + * of path if connection is binded to specific source.
>> + * @path:    The list of devices
>> + * @conn:    The connection of one outport
>> + *
>> + * Return zero if the connection doesn't have a source binded or 
>> source of the
>> + * path matches the source binds to connection.
>> + */
>> +static int coresight_source_filter(struct list_head *path,
>
> Instead of path, you may need to switch to "source" device. See below.
Got it. I will update in the next patch series.
>
>> +            struct coresight_connection *conn)
>> +{
>> +    int ret = 0;
>> +    struct coresight_device *source = NULL;
>> +
>> +    if (!conn->filter_src_dev)
>> +        return ret;
>> +
>> +    source = coresight_get_source(path);
>> +    if (!source)
>> +        return ret;
>> +
>> +    if (conn->filter_src_dev == source)
>> +        ret = 0;
>> +    else
>> +        ret = -1;
>> +
>> +    return ret;
>> +}
>> +
>>   static struct coresight_connection *
>>   coresight_find_out_connection(struct coresight_device *src_dev,
>> -                  struct coresight_device *dest_dev)
>> +                  struct coresight_device *dest_dev,
>> +                  struct list_head *path)
>>   {
>>       int i;
>>       struct coresight_connection *conn;
>>         for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>           conn = src_dev->pdata->out_conns[i];
>> +        if (coresight_source_filter(path, conn))
>> +            continue;
>>           if (conn->dest_dev == dest_dev)
>>               return conn;
>>       }
>> @@ -312,7 +359,8 @@ static void coresight_disable_sink(struct 
>> coresight_device *csdev)
>>     static int coresight_enable_link(struct coresight_device *csdev,
>>                    struct coresight_device *parent,
>> -                 struct coresight_device *child)
>> +                 struct coresight_device *child,
>> +                 struct list_head *path)
>>   {
>>       int ret = 0;
>>       int link_subtype;
>> @@ -321,8 +369,8 @@ static int coresight_enable_link(struct 
>> coresight_device *csdev,
>>       if (!parent || !child)
>>           return -EINVAL;
>>   -    inconn = coresight_find_out_connection(parent, csdev);
>> -    outconn = coresight_find_out_connection(csdev, child);
>> +    inconn = coresight_find_out_connection(parent, csdev, path);
>> +    outconn = coresight_find_out_connection(csdev, child, path);
>>       link_subtype = csdev->subtype.link_subtype;
>>         if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && 
>> IS_ERR(inconn))
>> @@ -341,7 +389,8 @@ static int coresight_enable_link(struct 
>> coresight_device *csdev,
>>     static void coresight_disable_link(struct coresight_device *csdev,
>>                      struct coresight_device *parent,
>> -                   struct coresight_device *child)
>> +                   struct coresight_device *child,
>> +                   struct list_head *path)
>>   {
>>       int i;
>>       int link_subtype;
>> @@ -350,8 +399,8 @@ static void coresight_disable_link(struct 
>> coresight_device *csdev,
>>       if (!parent || !child)
>>           return;
>>   -    inconn = coresight_find_out_connection(parent, csdev);
>> -    outconn = coresight_find_out_connection(csdev, child);
>> +    inconn = coresight_find_out_connection(parent, csdev, path);
>> +    outconn = coresight_find_out_connection(csdev, child, path);
>>       link_subtype = csdev->subtype.link_subtype;
>>         if (link_ops(csdev)->disable) {
>> @@ -507,7 +556,7 @@ static void coresight_disable_path_from(struct 
>> list_head *path,
>>           case CORESIGHT_DEV_TYPE_LINK:
>>               parent = list_prev_entry(nd, link)->csdev;
>>               child = list_next_entry(nd, link)->csdev;
>> -            coresight_disable_link(csdev, parent, child);
>> +            coresight_disable_link(csdev, parent, child, path);
>>               break;
>
> Disable path from could be called with a "partial path" with the
> "source" stripped off. e.g, if the enabling of the components failed
> mid-point in the path. So, source_from_path() above is WRONG csdev.
> Instead you should make sure we pass the "source", which must be
> available with the caller.
I will update in the next patch series.
>
>
>>           default:
>>               break;
>> @@ -588,7 +637,7 @@ int coresight_enable_path(struct list_head *path, 
>> enum cs_mode mode,
>>           case CORESIGHT_DEV_TYPE_LINK:
>>               parent = list_prev_entry(nd, link)->csdev;
>>               child = list_next_entry(nd, link)->csdev;
>> -            ret = coresight_enable_link(csdev, parent, child);
>> +            ret = coresight_enable_link(csdev, parent, child, path);
>>               if (ret)
>>                   goto err;
>>               break;
>> @@ -802,7 +851,8 @@ static void coresight_drop_device(struct 
>> coresight_device *csdev)
>>    */
>>   static int _coresight_build_path(struct coresight_device *csdev,
>>                    struct coresight_device *sink,
>> -                 struct list_head *path)
>> +                 struct list_head *path,
>> +                 struct coresight_device *trace_source)
>>   {
>>       int i, ret;
>>       bool found = false;
>> @@ -814,7 +864,7 @@ static int _coresight_build_path(struct 
>> coresight_device *csdev,
>>         if (coresight_is_percpu_source(csdev) && 
>> coresight_is_percpu_sink(sink) &&
>>           sink == per_cpu(csdev_sink, 
>> source_ops(csdev)->cpu_id(csdev))) {
>> -        if (_coresight_build_path(sink, sink, path) == 0) {
>> +        if (_coresight_build_path(sink, sink, path, trace_source) == 
>> 0) {
>>               found = true;
>>               goto out;
>>           }
>> @@ -825,8 +875,13 @@ static int _coresight_build_path(struct 
>> coresight_device *csdev,
>>           struct coresight_device *child_dev;
>>             child_dev = csdev->pdata->out_conns[i]->dest_dev;
>> +
>> +        if (csdev->pdata->out_conns[i]->filter_src_dev
>> +            && (csdev->pdata->out_conns[i]->filter_src_dev != 
>> trace_source))
>> +            continue;
>> +
>>           if (child_dev &&
>> -            _coresight_build_path(child_dev, sink, path) == 0) {
>> +            _coresight_build_path(child_dev, sink, path, 
>> trace_source) == 0) {
>>               found = true;
>>               break;
>>           }
>> @@ -871,7 +926,7 @@ struct list_head *coresight_build_path(struct 
>> coresight_device *source,
>>         INIT_LIST_HEAD(path);
>>   -    rc = _coresight_build_path(source, sink, path);
>> +    rc = _coresight_build_path(source, sink, path, source);
>>       if (rc) {
>>           kfree(path);
>>           return ERR_PTR(rc);
>> @@ -1395,6 +1450,9 @@ static int coresight_orphan_match(struct device 
>> *dev, void *data)
>>               /* This component still has an orphan */
>>               still_orphan = true;
>>           }
>> +        if ((conn->filter_src_fwnode) && dst_csdev
>> +            && (conn->filter_src_fwnode == dst_csdev->dev.fwnode))
>> +            conn->filter_src_dev = dst_csdev;
>>       }
>>         src_csdev->orphan = still_orphan;
>> @@ -1424,6 +1482,11 @@ static void coresight_remove_conns(struct 
>> coresight_device *csdev)
>>        */
>>       for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>>           conn = csdev->pdata->out_conns[i];
>> +        if (conn->filter_src_dev) {
>> +            conn->filter_src_dev = NULL;
>> +            conn->filter_src_fwnode = NULL;
>> +        }
>> +
>
> Similarly we should reset the "filter_src_dev" if the "src" csdev is
> being removed.  You may need a new function for that, which scans
> through all devices and looks for a conn->filter_src_dev == csdev.
> Something like:
I will update in the next patch series.
>
>
>
>>           if (!conn->dest_dev)
>>               continue;
>>   diff --git a/drivers/hwtracing/coresight/coresight-platform.c 
>> b/drivers/hwtracing/coresight/coresight-platform.c
>> index 9d550f5697fa..a9f5b0700310 100644
>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>> @@ -243,6 +243,19 @@ 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_src", 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);
>
> We should warn, if the filter_src_dev is of not the type DEV_TYPE_SOURCE.

I will update in the next patch series.


Best,

Tao

>
>> +
>>           new_conn = coresight_add_out_conn(dev, pdata, &conn);
>>           if (IS_ERR_VALUE(new_conn)) {
>>               fwnode_handle_put(conn.dest_fwnode);
>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>> index e8b6e388218c..0a7ec0978605 100644
>> --- a/include/linux/coresight.h
>> +++ b/include/linux/coresight.h
>> @@ -171,6 +171,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.
>> @@ -199,6 +202,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;
>>   };
>
> Suzuki
>

[PATCH v1 3/3] coresight-tpda: Optimize the function of reading element size

[Tao Zhang]

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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 52b0201090fb..fc5a4e46cf5d 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -110,9 +110,12 @@ static int tpda_get_element_size(struct tpda_drvdata *drvdata,
 		    csdev->pdata->in_conns[i]->dest_port != inport)
 			continue;
 
-		if (coresight_device_is_tpdm(in)) {
+		if (coresight_device_is_tpdm(in)
+		    || csdev->pdata->in_conns[i]->filter_src_dev) {
 			if (drvdata->dsb_esize || drvdata->cmb_esize)
 				return -EEXIST;
+			if (csdev->pdata->in_conns[i]->filter_src_dev)
+				in = csdev->pdata->in_conns[i]->filter_src_dev;
 			rc = tpdm_read_element_size(drvdata, in);
 			if (rc)
 				return rc;
@@ -124,7 +127,6 @@ static int tpda_get_element_size(struct tpda_drvdata *drvdata,
 		}
 	}
 
-
 	return rc;
 }
 
-- 
2.17.1

Re: [PATCH v1 3/3] coresight-tpda: Optimize the function of reading element size

[Suzuki K Poulose]

On 05/07/2024 09:51, Tao Zhang wrote:
> 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 | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> index 52b0201090fb..fc5a4e46cf5d 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -110,9 +110,12 @@ static int tpda_get_element_size(struct tpda_drvdata *drvdata,
>   		    csdev->pdata->in_conns[i]->dest_port != inport)
>   			continue;
>
		

> -		if (coresight_device_is_tpdm(in)) {
> +		if (coresight_device_is_tpdm(in)
> +		    || csdev->pdata->in_conns[i]->filter_src_dev) {

How can we assume that the filter-source device is always TPDM ?
It may be true for your test board, but driver must handle all
possible cases. I would rather prefer:

diff --git a/drivers/hwtracing/coresight/coresight-tpda.c 
b/drivers/hwtracing/coresight/coresight-tpda.c
index bfca103f9f84..b1d6f1dd60e3 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -110,6 +110,13 @@ static int tpda_get_element_size(struct 
tpda_drvdata *drvdata,
                     csdev->pdata->in_conns[i]->dest_port != inport)
                         continue;

+               /*
+                * If this port is tied to a source device,
+                * jump to that directly.
+                */
+               if (csdev->pdata->in_conns[i]->filter_src)
+                       in = csdev->pdata->in_conns[i]->filter_src;
+
                 if (coresight_device_is_tpdm(in)) {
                         if (drvdata->dsb_esize || drvdata->cmb_esize)
                                 return -EEXIST;


Suzuki

Re: [PATCH v1 3/3] coresight-tpda: Optimize the function of reading element size

[Tao Zhang]

On 7/8/2024 11:49 PM, Suzuki K Poulose wrote:
> On 05/07/2024 09:51, Tao Zhang wrote:
>> 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 | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c 
>> b/drivers/hwtracing/coresight/coresight-tpda.c
>> index 52b0201090fb..fc5a4e46cf5d 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>> @@ -110,9 +110,12 @@ static int tpda_get_element_size(struct 
>> tpda_drvdata *drvdata,
>>               csdev->pdata->in_conns[i]->dest_port != inport)
>>               continue;
>>
>
>
>> -        if (coresight_device_is_tpdm(in)) {
>> +        if (coresight_device_is_tpdm(in)
>> +            || csdev->pdata->in_conns[i]->filter_src_dev) {
>
> How can we assume that the filter-source device is always TPDM ?
> It may be true for your test board, but driver must handle all
> possible cases. I would rather prefer:

Got it. I will update in the next patch series.


Best,

Tao

>
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c 
> b/drivers/hwtracing/coresight/coresight-tpda.c
> index bfca103f9f84..b1d6f1dd60e3 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -110,6 +110,13 @@ static int tpda_get_element_size(struct 
> tpda_drvdata *drvdata,
>                     csdev->pdata->in_conns[i]->dest_port != inport)
>                         continue;
>
> +               /*
> +                * If this port is tied to a source device,
> +                * jump to that directly.
> +                */
> +               if (csdev->pdata->in_conns[i]->filter_src)
> +                       in = csdev->pdata->in_conns[i]->filter_src;
> +
>                 if (coresight_device_is_tpdm(in)) {
>                         if (drvdata->dsb_esize || drvdata->cmb_esize)
>                                 return -EEXIST;
>
>
> Suzuki
>