diff mbox series

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

Message ID 20240821031348.6837-2-quic_taozha@quicinc.com
State Under Review
Headers show
Series source filtering for multi-port output | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Tao Zhang Aug. 21, 2024, 3:13 a.m. UTC
The is some "magic" hard coded filtering in the replicators,
which only passes through trace from a particular "source". Add
a new property "filter-src" 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>
---
 .../arm/arm,coresight-static-replicator.yaml  | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Comments

Suzuki K Poulose Aug. 21, 2024, 10:38 a.m. UTC | #1
On 21/08/2024 04:13, Tao Zhang wrote:
> The is some "magic" hard coded filtering in the replicators,
> which only passes through trace from a particular "source". Add
> a new property "filter-src" to label a phandle to the coresight
> trace source device matching the hard coded filtering for the port.

Minor nit: Please do not use abbreviate "source" in the bindings.
I am not an expert on other changes below and will leave it to
Rob/Krzysztof to comment.

Rob, Krzysztof,

We need someway to "link" (add a phandle) from a "port". The patch below
is extending "standard" port to add a phandle. Please let us know if
there is a better way.

e.g.:

filters = list of tuples of port, phandle. ?

e.g.:

filters = < 0, <&tpdm_video>,
             1, <&tpdm_mdss>
	   >


Thanks

Suzuki


> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   .../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..0d258c79eb94 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-src:
> +                $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-src = <&tpdm_video>;
>                   };
>               };
>   
> @@ -79,6 +95,7 @@ examples:
>                   reg = <1>;
>                   replicator_out_port1: endpoint {
>                       remote-endpoint = <&tpiu_in_port>;
> +                    filter-src = <&tpdm_mdss>;
>                   };
>               };
>           };
Suzuki K Poulose Aug. 22, 2024, 10:34 a.m. UTC | #2
On 22/08/2024 08:08, Krzysztof Kozlowski wrote:
> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote:
>> On 21/08/2024 04:13, Tao Zhang wrote:
>>> The is some "magic" hard coded filtering in the replicators,
>>> which only passes through trace from a particular "source". Add
>>> a new property "filter-src" to label a phandle to the coresight
>>> trace source device matching the hard coded filtering for the port.
>>
>> Minor nit: Please do not use abbreviate "source" in the bindings.
>> I am not an expert on other changes below and will leave it to
>> Rob/Krzysztof to comment.
>>
>> Rob, Krzysztof,
>>
>> We need someway to "link" (add a phandle) from a "port". The patch below
>> is extending "standard" port to add a phandle. Please let us know if
>> there is a better way.
>>
>> e.g.:
>>
>> filters = list of tuples of port, phandle. ?
>>
>> e.g.:
>>
>> filters = < 0, <&tpdm_video>,
>>              1, <&tpdm_mdss>
>> 	   >
>>
> 
> Current solution feels like band-aid - what if next time you need some
> second filter? Or "wall"? Or whatever? Next property?



> 
> Isn't filter just one endpoint in the graph?
> 
> A <--> filter <--> B

To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a
multi output ports).

For clearer example:

A0 <--> .. <--> ..\                  p0  / --> Filtered for (A1) <--> B1
A1 <--> .. <--> .. - < L(filters>    p1  - --> Filtered for (A2) <--> B2
A2 <--> .. <--> ../                  p2  \ --> Unfiltered        <--> B0



> Instead of
> 
> A <----through-filter----> B?

The problem is we need to know the components in the path from A0 to X
through, (Not just A0 and L). And also we need to know "which port (p0 
vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out of the
link "L".

So ideally, we need a way to tie p0 -> A1, p1 -> A2.

would we need something else in the future ? I don't know for sure.
People could design their own things ;-). But this was the first time
ever in the last 12yrs since we supported coresight in the kernel.
(there is always a first time).

Fundamentally, the "ports" cannot have additional properties today.
Not sure if there are other usecases (I don't see why). So, we have
to manually extend like above, which I think is not nice.

Happy to proceed with anything that seems acceptable for you folks.

Suzuki



> 
> Best regards,
> Krzysztof
>
Suzuki K Poulose Aug. 22, 2024, 11:50 a.m. UTC | #3
On 22/08/2024 11:34, Suzuki K Poulose wrote:
> On 22/08/2024 08:08, Krzysztof Kozlowski wrote:
>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote:
>>> On 21/08/2024 04:13, Tao Zhang wrote:
>>>> The is some "magic" hard coded filtering in the replicators,
>>>> which only passes through trace from a particular "source". Add
>>>> a new property "filter-src" to label a phandle to the coresight
>>>> trace source device matching the hard coded filtering for the port.
>>>
>>> Minor nit: Please do not use abbreviate "source" in the bindings.
>>> I am not an expert on other changes below and will leave it to
>>> Rob/Krzysztof to comment.
>>>
>>> Rob, Krzysztof,
>>>
>>> We need someway to "link" (add a phandle) from a "port". The patch below
>>> is extending "standard" port to add a phandle. Please let us know if
>>> there is a better way.
>>>
>>> e.g.:
>>>
>>> filters = list of tuples of port, phandle. ?
>>>
>>> e.g.:
>>>
>>> filters = < 0, <&tpdm_video>,
>>>              1, <&tpdm_mdss>
>>>        >
>>>
>>
>> Current solution feels like band-aid - what if next time you need some
>> second filter? Or "wall"? Or whatever? Next property?
> 
> 
> 
>>
>> Isn't filter just one endpoint in the graph?
>>
>> A <--> filter <--> B
> 
> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a
> multi output ports).
> 
> For clearer example:
> 
> A0 <--> .. <--> ..\                  p0  / --> Filtered for (A1) <--> B1
> A1 <--> .. <--> .. - < L(filters>    p1  - --> Filtered for (A2) <--> B2
> A2 <--> .. <--> ../                  p2  \ --> Unfiltered        <--> B0
> 
> 
> 
>> Instead of
>>
>> A <----through-filter----> B?
> 
> The problem is we need to know the components in the path from A0 to X
> through, (Not just A0 and L). And also we need to know "which port (p0 
> vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out of the
> link "L".
> 
> So ideally, we need a way to tie p0 -> A1, p1 -> A2.
> 
> would we need something else in the future ? I don't know for sure.
> People could design their own things ;-). But this was the first time
> ever in the last 12yrs since we supported coresight in the kernel.
> (there is always a first time).
> 
> Fundamentally, the "ports" cannot have additional properties today.
> Not sure if there are other usecases (I don't see why). So, we have
> to manually extend like above, which I think is not nice.

Replying to the other thread [0], made me realize that the above is not
true. Indeed it is possible to add properties for endpoints, e.g:

e.g.: media/video-interfaces.yaml

So extending the endpoint node is indeed acceptable (unlike I thought).
May be the we it is achieved in this patch is making it look otherwise.

Suzuki
[0] https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com



> 
> Happy to proceed with anything that seems acceptable for you folks.
> 
> Suzuki
> 
> 
> 
>>
>> Best regards,
>> Krzysztof
>>
>
Suzuki K Poulose Oct. 9, 2024, 10:52 a.m. UTC | #4
Krzysztof

On 22/08/2024 12:50, Suzuki K Poulose wrote:
> On 22/08/2024 11:34, Suzuki K Poulose wrote:
>> On 22/08/2024 08:08, Krzysztof Kozlowski wrote:
>>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote:
>>>> On 21/08/2024 04:13, Tao Zhang wrote:
>>>>> The is some "magic" hard coded filtering in the replicators,
>>>>> which only passes through trace from a particular "source". Add
>>>>> a new property "filter-src" to label a phandle to the coresight
>>>>> trace source device matching the hard coded filtering for the port.
>>>>
>>>> Minor nit: Please do not use abbreviate "source" in the bindings.
>>>> I am not an expert on other changes below and will leave it to
>>>> Rob/Krzysztof to comment.
>>>>
>>>> Rob, Krzysztof,
>>>>
>>>> We need someway to "link" (add a phandle) from a "port". The patch 
>>>> below
>>>> is extending "standard" port to add a phandle. Please let us know if
>>>> there is a better way.
>>>>
>>>> e.g.:
>>>>
>>>> filters = list of tuples of port, phandle. ?
>>>>
>>>> e.g.:
>>>>
>>>> filters = < 0, <&tpdm_video>,
>>>>              1, <&tpdm_mdss>
>>>>        >
>>>>
>>>
>>> Current solution feels like band-aid - what if next time you need some
>>> second filter? Or "wall"? Or whatever? Next property?
>>
>>
>>
>>>
>>> Isn't filter just one endpoint in the graph?
>>>
>>> A <--> filter <--> B
>>
>> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a
>> multi output ports).
>>
>> For clearer example:
>>
>> A0 <--> .. <--> ..\                  p0  / --> Filtered for (A1) <--> B1
>> A1 <--> .. <--> .. - < L(filters>    p1  - --> Filtered for (A2) <--> B2
>> A2 <--> .. <--> ../                  p2  \ --> Unfiltered        <--> B0
>>
>>
>>
>>> Instead of
>>>
>>> A <----through-filter----> B?
>>
>> The problem is we need to know the components in the path from A0 to X
>> through, (Not just A0 and L). And also we need to know "which port (p0 
>> vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out of the
>> link "L".
>>
>> So ideally, we need a way to tie p0 -> A1, p1 -> A2.
>>
>> would we need something else in the future ? I don't know for sure.
>> People could design their own things ;-). But this was the first time
>> ever in the last 12yrs since we supported coresight in the kernel.
>> (there is always a first time).
>>
>> Fundamentally, the "ports" cannot have additional properties today.
>> Not sure if there are other usecases (I don't see why). So, we have
>> to manually extend like above, which I think is not nice.
> 
> Replying to the other thread [0], made me realize that the above is not
> true. Indeed it is possible to add properties for endpoints, e.g:
> 
> e.g.: media/video-interfaces.yaml
> 
> So extending the endpoint node is indeed acceptable (unlike I thought).
> May be the we it is achieved in this patch is making it look otherwise.
> 
> Suzuki
> [0] https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com

Please could you let us know if it is acceptable to extend "endpoint"
node to have an optional property ?

Suzuki


> 
> 
> 
>>
>> Happy to proceed with anything that seems acceptable for you folks.
>>
>> Suzuki
>>
>>
>>
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>
Tao Zhang Oct. 17, 2024, 7:23 a.m. UTC | #5
On 10/9/2024 6:52 PM, Suzuki K Poulose wrote:
> Krzysztof
>
> On 22/08/2024 12:50, Suzuki K Poulose wrote:
>> On 22/08/2024 11:34, Suzuki K Poulose wrote:
>>> On 22/08/2024 08:08, Krzysztof Kozlowski wrote:
>>>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote:
>>>>> On 21/08/2024 04:13, Tao Zhang wrote:
>>>>>> The is some "magic" hard coded filtering in the replicators,
>>>>>> which only passes through trace from a particular "source". Add
>>>>>> a new property "filter-src" to label a phandle to the coresight
>>>>>> trace source device matching the hard coded filtering for the port.
>>>>>
>>>>> Minor nit: Please do not use abbreviate "source" in the bindings.
>>>>> I am not an expert on other changes below and will leave it to
>>>>> Rob/Krzysztof to comment.
>>>>>
>>>>> Rob, Krzysztof,
>>>>>
>>>>> We need someway to "link" (add a phandle) from a "port". The patch 
>>>>> below
>>>>> is extending "standard" port to add a phandle. Please let us know if
>>>>> there is a better way.
>>>>>
>>>>> e.g.:
>>>>>
>>>>> filters = list of tuples of port, phandle. ?
>>>>>
>>>>> e.g.:
>>>>>
>>>>> filters = < 0, <&tpdm_video>,
>>>>>              1, <&tpdm_mdss>
>>>>>        >
>>>>>
>>>>
>>>> Current solution feels like band-aid - what if next time you need some
>>>> second filter? Or "wall"? Or whatever? Next property?
>>>
>>>
>>>
>>>>
>>>> Isn't filter just one endpoint in the graph?
>>>>
>>>> A <--> filter <--> B
>>>
>>> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a
>>> multi output ports).
>>>
>>> For clearer example:
>>>
>>> A0 <--> .. <--> ..\                  p0  / --> Filtered for (A1) 
>>> <--> B1
>>> A1 <--> .. <--> .. - < L(filters>    p1  - --> Filtered for (A2) 
>>> <--> B2
>>> A2 <--> .. <--> ../                  p2  \ --> Unfiltered        
>>> <--> B0
>>>
>>>
>>>
>>>> Instead of
>>>>
>>>> A <----through-filter----> B?
>>>
>>> The problem is we need to know the components in the path from A0 to X
>>> through, (Not just A0 and L). And also we need to know "which port 
>>> (p0 vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out 
>>> of the
>>> link "L".
>>>
>>> So ideally, we need a way to tie p0 -> A1, p1 -> A2.
>>>
>>> would we need something else in the future ? I don't know for sure.
>>> People could design their own things ;-). But this was the first time
>>> ever in the last 12yrs since we supported coresight in the kernel.
>>> (there is always a first time).
>>>
>>> Fundamentally, the "ports" cannot have additional properties today.
>>> Not sure if there are other usecases (I don't see why). So, we have
>>> to manually extend like above, which I think is not nice.
>>
>> Replying to the other thread [0], made me realize that the above is not
>> true. Indeed it is possible to add properties for endpoints, e.g:
>>
>> e.g.: media/video-interfaces.yaml
>>
>> So extending the endpoint node is indeed acceptable (unlike I thought).
>> May be the we it is achieved in this patch is making it look otherwise.
>>
>> Suzuki
>> [0] 
>> https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com
>
> Please could you let us know if it is acceptable to extend "endpoint"
> node to have an optional property ?

Hi Krzysztof,


Kindly reminder, could you help comment on this?


Best,

Tao

>
> Suzuki
>
>
>>
>>
>>
>>>
>>> Happy to proceed with anything that seems acceptable for you folks.
>>>
>>> Suzuki
>>>
>>>
>>>
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
Krzysztof Kozlowski Oct. 18, 2024, 10:05 a.m. UTC | #6
On 17/10/2024 09:23, Tao Zhang wrote:
> 
> On 10/9/2024 6:52 PM, Suzuki K Poulose wrote:
>> Krzysztof
>>
>> On 22/08/2024 12:50, Suzuki K Poulose wrote:
>>> On 22/08/2024 11:34, Suzuki K Poulose wrote:
>>>> On 22/08/2024 08:08, Krzysztof Kozlowski wrote:
>>>>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote:
>>>>>> On 21/08/2024 04:13, Tao Zhang wrote:
>>>>>>> The is some "magic" hard coded filtering in the replicators,
>>>>>>> which only passes through trace from a particular "source". Add
>>>>>>> a new property "filter-src" to label a phandle to the coresight
>>>>>>> trace source device matching the hard coded filtering for the port.
>>>>>>
>>>>>> Minor nit: Please do not use abbreviate "source" in the bindings.
>>>>>> I am not an expert on other changes below and will leave it to
>>>>>> Rob/Krzysztof to comment.
>>>>>>
>>>>>> Rob, Krzysztof,
>>>>>>
>>>>>> We need someway to "link" (add a phandle) from a "port". The patch 
>>>>>> below
>>>>>> is extending "standard" port to add a phandle. Please let us know if
>>>>>> there is a better way.
>>>>>>
>>>>>> e.g.:
>>>>>>
>>>>>> filters = list of tuples of port, phandle. ?
>>>>>>
>>>>>> e.g.:
>>>>>>
>>>>>> filters = < 0, <&tpdm_video>,
>>>>>>              1, <&tpdm_mdss>
>>>>>>        >
>>>>>>
>>>>>
>>>>> Current solution feels like band-aid - what if next time you need some
>>>>> second filter? Or "wall"? Or whatever? Next property?
>>>>
>>>>
>>>>
>>>>>
>>>>> Isn't filter just one endpoint in the graph?
>>>>>
>>>>> A <--> filter <--> B
>>>>
>>>> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a
>>>> multi output ports).
>>>>
>>>> For clearer example:
>>>>
>>>> A0 <--> .. <--> ..\                  p0  / --> Filtered for (A1) 
>>>> <--> B1
>>>> A1 <--> .. <--> .. - < L(filters>    p1  - --> Filtered for (A2) 
>>>> <--> B2
>>>> A2 <--> .. <--> ../                  p2  \ --> Unfiltered        
>>>> <--> B0
>>>>
>>>>
>>>>
>>>>> Instead of
>>>>>
>>>>> A <----through-filter----> B?
>>>>
>>>> The problem is we need to know the components in the path from A0 to X
>>>> through, (Not just A0 and L). And also we need to know "which port 
>>>> (p0 vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out 
>>>> of the
>>>> link "L".
>>>>
>>>> So ideally, we need a way to tie p0 -> A1, p1 -> A2.
>>>>
>>>> would we need something else in the future ? I don't know for sure.
>>>> People could design their own things ;-). But this was the first time
>>>> ever in the last 12yrs since we supported coresight in the kernel.
>>>> (there is always a first time).
>>>>
>>>> Fundamentally, the "ports" cannot have additional properties today.
>>>> Not sure if there are other usecases (I don't see why). So, we have
>>>> to manually extend like above, which I think is not nice.
>>>
>>> Replying to the other thread [0], made me realize that the above is not
>>> true. Indeed it is possible to add properties for endpoints, e.g:
>>>
>>> e.g.: media/video-interfaces.yaml
>>>
>>> So extending the endpoint node is indeed acceptable (unlike I thought).
>>> May be the we it is achieved in this patch is making it look otherwise.
>>>
>>> Suzuki
>>> [0] 
>>> https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com
>>
>> Please could you let us know if it is acceptable to extend "endpoint"
>> node to have an optional property ?
> 
> Hi Krzysztof,
> 
> 
> Kindly reminder, could you help comment on this?

I don't have any smart ideas and with earlier explanation sounds ok.

Best regards,
Krzysztof
Suzuki K Poulose Oct. 18, 2024, 10:08 a.m. UTC | #7
On 18/10/2024 11:05, Krzysztof Kozlowski wrote:
> On 17/10/2024 09:23, Tao Zhang wrote:
>>
>> On 10/9/2024 6:52 PM, Suzuki K Poulose wrote:
>>> Krzysztof
>>>
>>> On 22/08/2024 12:50, Suzuki K Poulose wrote:
>>>> On 22/08/2024 11:34, Suzuki K Poulose wrote:
>>>>> On 22/08/2024 08:08, Krzysztof Kozlowski wrote:
>>>>>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote:
>>>>>>> On 21/08/2024 04:13, Tao Zhang wrote:
>>>>>>>> The is some "magic" hard coded filtering in the replicators,
>>>>>>>> which only passes through trace from a particular "source". Add
>>>>>>>> a new property "filter-src" to label a phandle to the coresight
>>>>>>>> trace source device matching the hard coded filtering for the port.
>>>>>>>
>>>>>>> Minor nit: Please do not use abbreviate "source" in the bindings.
>>>>>>> I am not an expert on other changes below and will leave it to
>>>>>>> Rob/Krzysztof to comment.
>>>>>>>
>>>>>>> Rob, Krzysztof,
>>>>>>>
>>>>>>> We need someway to "link" (add a phandle) from a "port". The patch
>>>>>>> below
>>>>>>> is extending "standard" port to add a phandle. Please let us know if
>>>>>>> there is a better way.
>>>>>>>
>>>>>>> e.g.:
>>>>>>>
>>>>>>> filters = list of tuples of port, phandle. ?
>>>>>>>
>>>>>>> e.g.:
>>>>>>>
>>>>>>> filters = < 0, <&tpdm_video>,
>>>>>>>               1, <&tpdm_mdss>
>>>>>>>         >
>>>>>>>
>>>>>>
>>>>>> Current solution feels like band-aid - what if next time you need some
>>>>>> second filter? Or "wall"? Or whatever? Next property?
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Isn't filter just one endpoint in the graph?
>>>>>>
>>>>>> A <--> filter <--> B
>>>>>
>>>>> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a
>>>>> multi output ports).
>>>>>
>>>>> For clearer example:
>>>>>
>>>>> A0 <--> .. <--> ..\                  p0  / --> Filtered for (A1)
>>>>> <--> B1
>>>>> A1 <--> .. <--> .. - < L(filters>    p1  - --> Filtered for (A2)
>>>>> <--> B2
>>>>> A2 <--> .. <--> ../                  p2  \ --> Unfiltered
>>>>> <--> B0
>>>>>
>>>>>
>>>>>
>>>>>> Instead of
>>>>>>
>>>>>> A <----through-filter----> B?
>>>>>
>>>>> The problem is we need to know the components in the path from A0 to X
>>>>> through, (Not just A0 and L). And also we need to know "which port
>>>>> (p0 vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out
>>>>> of the
>>>>> link "L".
>>>>>
>>>>> So ideally, we need a way to tie p0 -> A1, p1 -> A2.
>>>>>
>>>>> would we need something else in the future ? I don't know for sure.
>>>>> People could design their own things ;-). But this was the first time
>>>>> ever in the last 12yrs since we supported coresight in the kernel.
>>>>> (there is always a first time).
>>>>>
>>>>> Fundamentally, the "ports" cannot have additional properties today.
>>>>> Not sure if there are other usecases (I don't see why). So, we have
>>>>> to manually extend like above, which I think is not nice.
>>>>
>>>> Replying to the other thread [0], made me realize that the above is not
>>>> true. Indeed it is possible to add properties for endpoints, e.g:
>>>>
>>>> e.g.: media/video-interfaces.yaml
>>>>
>>>> So extending the endpoint node is indeed acceptable (unlike I thought).
>>>> May be the we it is achieved in this patch is making it look otherwise.
>>>>
>>>> Suzuki
>>>> [0]
>>>> https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com
>>>
>>> Please could you let us know if it is acceptable to extend "endpoint"
>>> node to have an optional property ?
>>
>> Hi Krzysztof,
>>
>>
>> Kindly reminder, could you help comment on this?
> 
> I don't have any smart ideas and with earlier explanation sounds ok.

Just to confirm, are you OK with adding a property to the "endpoint"
node that will indicate a phandle that the device allows on this
endpoint ?

Kind regards
Suzuki


> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Oct. 18, 2024, 10:31 a.m. UTC | #8
On 18/10/2024 12:08, Suzuki K Poulose wrote:
> On 18/10/2024 11:05, Krzysztof Kozlowski wrote:
>> On 17/10/2024 09:23, Tao Zhang wrote:
>>>
>>> On 10/9/2024 6:52 PM, Suzuki K Poulose wrote:
>>>> Krzysztof
>>>>
>>>> On 22/08/2024 12:50, Suzuki K Poulose wrote:
>>>>> On 22/08/2024 11:34, Suzuki K Poulose wrote:
>>>>>> On 22/08/2024 08:08, Krzysztof Kozlowski wrote:
>>>>>>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote:
>>>>>>>> On 21/08/2024 04:13, Tao Zhang wrote:
>>>>>>>>> The is some "magic" hard coded filtering in the replicators,
>>>>>>>>> which only passes through trace from a particular "source". Add
>>>>>>>>> a new property "filter-src" to label a phandle to the coresight
>>>>>>>>> trace source device matching the hard coded filtering for the port.
>>>>>>>>
>>>>>>>> Minor nit: Please do not use abbreviate "source" in the bindings.
>>>>>>>> I am not an expert on other changes below and will leave it to
>>>>>>>> Rob/Krzysztof to comment.
>>>>>>>>
>>>>>>>> Rob, Krzysztof,
>>>>>>>>
>>>>>>>> We need someway to "link" (add a phandle) from a "port". The patch
>>>>>>>> below
>>>>>>>> is extending "standard" port to add a phandle. Please let us know if
>>>>>>>> there is a better way.
>>>>>>>>
>>>>>>>> e.g.:
>>>>>>>>
>>>>>>>> filters = list of tuples of port, phandle. ?
>>>>>>>>
>>>>>>>> e.g.:
>>>>>>>>
>>>>>>>> filters = < 0, <&tpdm_video>,
>>>>>>>>               1, <&tpdm_mdss>
>>>>>>>>         >
>>>>>>>>
>>>>>>>
>>>>>>> Current solution feels like band-aid - what if next time you need some
>>>>>>> second filter? Or "wall"? Or whatever? Next property?
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Isn't filter just one endpoint in the graph?
>>>>>>>
>>>>>>> A <--> filter <--> B
>>>>>>
>>>>>> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a
>>>>>> multi output ports).
>>>>>>
>>>>>> For clearer example:
>>>>>>
>>>>>> A0 <--> .. <--> ..\                  p0  / --> Filtered for (A1)
>>>>>> <--> B1
>>>>>> A1 <--> .. <--> .. - < L(filters>    p1  - --> Filtered for (A2)
>>>>>> <--> B2
>>>>>> A2 <--> .. <--> ../                  p2  \ --> Unfiltered
>>>>>> <--> B0
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Instead of
>>>>>>>
>>>>>>> A <----through-filter----> B?
>>>>>>
>>>>>> The problem is we need to know the components in the path from A0 to X
>>>>>> through, (Not just A0 and L). And also we need to know "which port
>>>>>> (p0 vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out
>>>>>> of the
>>>>>> link "L".
>>>>>>
>>>>>> So ideally, we need a way to tie p0 -> A1, p1 -> A2.
>>>>>>
>>>>>> would we need something else in the future ? I don't know for sure.
>>>>>> People could design their own things ;-). But this was the first time
>>>>>> ever in the last 12yrs since we supported coresight in the kernel.
>>>>>> (there is always a first time).
>>>>>>
>>>>>> Fundamentally, the "ports" cannot have additional properties today.
>>>>>> Not sure if there are other usecases (I don't see why). So, we have
>>>>>> to manually extend like above, which I think is not nice.
>>>>>
>>>>> Replying to the other thread [0], made me realize that the above is not
>>>>> true. Indeed it is possible to add properties for endpoints, e.g:
>>>>>
>>>>> e.g.: media/video-interfaces.yaml
>>>>>
>>>>> So extending the endpoint node is indeed acceptable (unlike I thought).
>>>>> May be the we it is achieved in this patch is making it look otherwise.
>>>>>
>>>>> Suzuki
>>>>> [0]
>>>>> https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com
>>>>
>>>> Please could you let us know if it is acceptable to extend "endpoint"
>>>> node to have an optional property ?
>>>
>>> Hi Krzysztof,
>>>
>>>
>>> Kindly reminder, could you help comment on this?
>>
>> I don't have any smart ideas and with earlier explanation sounds ok.
> 
> Just to confirm, are you OK with adding a property to the "endpoint"
> node that will indicate a phandle that the device allows on this
> endpoint ?

You mean the filter property in endpoint? if so, then yes.

Best regards,
Krzysztof
Suzuki K Poulose Oct. 18, 2024, 10:47 a.m. UTC | #9
On 18/10/2024 11:31, Krzysztof Kozlowski wrote:
> On 18/10/2024 12:08, Suzuki K Poulose wrote:
>> On 18/10/2024 11:05, Krzysztof Kozlowski wrote:
>>> On 17/10/2024 09:23, Tao Zhang wrote:
>>>>
>>>> On 10/9/2024 6:52 PM, Suzuki K Poulose wrote:
>>>>> Krzysztof
>>>>>
>>>>> On 22/08/2024 12:50, Suzuki K Poulose wrote:
>>>>>> On 22/08/2024 11:34, Suzuki K Poulose wrote:
>>>>>>> On 22/08/2024 08:08, Krzysztof Kozlowski wrote:
>>>>>>>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote:
>>>>>>>>> On 21/08/2024 04:13, Tao Zhang wrote:
>>>>>>>>>> The is some "magic" hard coded filtering in the replicators,
>>>>>>>>>> which only passes through trace from a particular "source". Add
>>>>>>>>>> a new property "filter-src" to label a phandle to the coresight
>>>>>>>>>> trace source device matching the hard coded filtering for the port.
>>>>>>>>>
>>>>>>>>> Minor nit: Please do not use abbreviate "source" in the bindings.
>>>>>>>>> I am not an expert on other changes below and will leave it to
>>>>>>>>> Rob/Krzysztof to comment.
>>>>>>>>>
>>>>>>>>> Rob, Krzysztof,
>>>>>>>>>
>>>>>>>>> We need someway to "link" (add a phandle) from a "port". The patch
>>>>>>>>> below
>>>>>>>>> is extending "standard" port to add a phandle. Please let us know if
>>>>>>>>> there is a better way.
>>>>>>>>>
>>>>>>>>> e.g.:
>>>>>>>>>
>>>>>>>>> filters = list of tuples of port, phandle. ?
>>>>>>>>>
>>>>>>>>> e.g.:
>>>>>>>>>
>>>>>>>>> filters = < 0, <&tpdm_video>,
>>>>>>>>>                1, <&tpdm_mdss>
>>>>>>>>>          >
>>>>>>>>>
>>>>>>>>
>>>>>>>> Current solution feels like band-aid - what if next time you need some
>>>>>>>> second filter? Or "wall"? Or whatever? Next property?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Isn't filter just one endpoint in the graph?
>>>>>>>>
>>>>>>>> A <--> filter <--> B
>>>>>>>
>>>>>>> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a
>>>>>>> multi output ports).
>>>>>>>
>>>>>>> For clearer example:
>>>>>>>
>>>>>>> A0 <--> .. <--> ..\                  p0  / --> Filtered for (A1)
>>>>>>> <--> B1
>>>>>>> A1 <--> .. <--> .. - < L(filters>    p1  - --> Filtered for (A2)
>>>>>>> <--> B2
>>>>>>> A2 <--> .. <--> ../                  p2  \ --> Unfiltered
>>>>>>> <--> B0
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> Instead of
>>>>>>>>
>>>>>>>> A <----through-filter----> B?
>>>>>>>
>>>>>>> The problem is we need to know the components in the path from A0 to X
>>>>>>> through, (Not just A0 and L). And also we need to know "which port
>>>>>>> (p0 vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out
>>>>>>> of the
>>>>>>> link "L".
>>>>>>>
>>>>>>> So ideally, we need a way to tie p0 -> A1, p1 -> A2.
>>>>>>>
>>>>>>> would we need something else in the future ? I don't know for sure.
>>>>>>> People could design their own things ;-). But this was the first time
>>>>>>> ever in the last 12yrs since we supported coresight in the kernel.
>>>>>>> (there is always a first time).
>>>>>>>
>>>>>>> Fundamentally, the "ports" cannot have additional properties today.
>>>>>>> Not sure if there are other usecases (I don't see why). So, we have
>>>>>>> to manually extend like above, which I think is not nice.
>>>>>>
>>>>>> Replying to the other thread [0], made me realize that the above is not
>>>>>> true. Indeed it is possible to add properties for endpoints, e.g:
>>>>>>
>>>>>> e.g.: media/video-interfaces.yaml
>>>>>>
>>>>>> So extending the endpoint node is indeed acceptable (unlike I thought).
>>>>>> May be the we it is achieved in this patch is making it look otherwise.
>>>>>>
>>>>>> Suzuki
>>>>>> [0]
>>>>>> https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com
>>>>>
>>>>> Please could you let us know if it is acceptable to extend "endpoint"
>>>>> node to have an optional property ?
>>>>
>>>> Hi Krzysztof,
>>>>
>>>>
>>>> Kindly reminder, could you help comment on this?
>>>
>>> I don't have any smart ideas and with earlier explanation sounds ok.
>>
>> Just to confirm, are you OK with adding a property to the "endpoint"
>> node that will indicate a phandle that the device allows on this
>> endpoint ?
> 
> You mean the filter property in endpoint? if so, then yes.

Thanks for confirming !

Cheers
Suzuki


> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
index 1892a091ac35..0d258c79eb94 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-src:
+                $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-src = <&tpdm_video>;
                 };
             };
 
@@ -79,6 +95,7 @@  examples:
                 reg = <1>;
                 replicator_out_port1: endpoint {
                     remote-endpoint = <&tpiu_in_port>;
+                    filter-src = <&tpdm_mdss>;
                 };
             };
         };