Message ID | 20240821031348.6837-2-quic_taozha@quicinc.com (mailing list archive) |
---|---|
State | Not Applicable |
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 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 >> >
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(-)