| Message ID | 20240821031348.6837-2-quic_taozha@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | source filtering for multi-port output | expand |
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>; > }; > }; > };
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 Instead of A <----through-filter----> B? Best regards, Krzysztof
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 >
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 >> >
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 >>> >> >
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
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
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 >
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
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 --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>; }; }; };
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(-)