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

[Suzuki K Poulose <suzuki.poulose@arm.com>]

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
>