diff mbox series

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

Message ID 20240705085152.9063-4-quic_taozha@quicinc.com
State New
Headers show
Series [v1,1/3] dt-bindings: arm: qcom,coresight-static-replicator: Add property for source filtering | expand

Commit Message

Tao Zhang July 5, 2024, 8:51 a.m. UTC
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(-)

Comments

Suzuki K Poulose July 8, 2024, 3:49 p.m. UTC | #1
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
Tao Zhang July 11, 2024, 8:20 a.m. UTC | #2
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
>
diff mbox series

Patch

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;
 }