| Message ID | 20240705090049.1656986-3-quic_jiegan@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Coresight: Add Coresight Control Unit driver | expand |
On 05/07/2024 11:00, Jie Gan wrote: > Add binding document for Coresight Control Unit device. <form letter> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC (and consider --no-git-fallback argument). It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. </form letter> Or stop developing on some old tree. It's some sort of weird pattern in entire Qualcomm Coresight - everything developed on old kernels. You must work on latest mainline or maintainer or linux-next tree, not some old Qualcomm tree. Your v5.15, v5.19, v6.4 or v6.8 or whatever you have there: BIG NOPE. > > Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> > --- Subject: it never ends with full stop. A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > .../bindings/arm/qcom,coresight-ccu.yaml | 87 +++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > > diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > new file mode 100644 > index 000000000000..9bb8ced393a7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > @@ -0,0 +1,87 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/arm/qcom,coresight-ccu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: CoreSight Control Unit > + > +maintainers: > + - Yuanfang Zhang <quic_yuanfang@quicinc.com> > + - Mao Jinlong <quic_jinlmao@quicinc.com> > + - Jie Gan <quic_jiegan@quicinc.com> > + > +description: > + The Coresight Control unit controls various Coresight behaviors. > + Used to enable/disable ETR’s data filter function based on trace ID. > + > +properties: > + compatible: > + const: qcom,coresight-ccu > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: apb_pclk Drop _pclk > + > + reg-names: Please follow DTS coding style about order of properties. > + items: > + - const: ccu-base > + > + in-ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + unevaluatedProperties: This was never tested and it cannot reliably work. Sorry, this is waste of our time. > + patternProperties: > + '^port(@[0-7])?$': > + description: Input connections from CoreSight Trace bus > + $ref: /schemas/graph.yaml#/properties/port > + > + properties: > + qcom,ccu-atid-offset: > + description: > + Offset to the Coresight Control Unit component's ATID register > + that is used by specific TMC ETR. The ATID register can be programed based > + on the trace id to filter out specific trace data which gets into ETR buffer. > + $ref: /schemas/types.yaml#/definitions/uint32 > + > +required: > + - compatible > + - reg > + - in-ports > + > +additionalProperties: false > + > +examples: > + - | > + syscon@1001000 { That's not a syscon. > + compatible = "qcom,coresight-ccu"; > + reg = <0x1001000 0x1000>; > + reg-names = "ccu-base"; > + Best regards, Krzysztof
On Fri, 05 Jul 2024 17:00:47 +0800, Jie Gan wrote: > Add binding document for Coresight Control Unit device. > > Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> > --- > .../bindings/arm/qcom,coresight-ccu.yaml | 87 +++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml: unevaluatedProperties: Missing additionalProperties/unevaluatedProperties constraint doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240705090049.1656986-3-quic_jiegan@quicinc.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Fri, Jul 05, 2024 at 11:07:22AM +0200, Krzysztof Kozlowski wrote: > On 05/07/2024 11:00, Jie Gan wrote: > > Add binding document for Coresight Control Unit device. > > <form letter> > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC (and consider --no-git-fallback argument). It might > happen, that command when run on an older kernel, gives you outdated > entries. Therefore please be sure you base your patches on recent Linux > kernel. > > Tools like b4 or scripts/get_maintainer.pl provide you proper list of > people, so fix your workflow. Tools might also fail if you work on some > ancient tree (don't, instead use mainline) or work on fork of kernel > (don't, instead use mainline). Just use b4 and everything should be > fine, although remember about `b4 prep --auto-to-cc` if you added new > patches to the patchset. > </form letter> > > Or stop developing on some old tree. It's some sort of weird pattern in > entire Qualcomm Coresight - everything developed on old kernels. > > You must work on latest mainline or maintainer or linux-next tree, not > some old Qualcomm tree. Your v5.15, v5.19, v6.4 or v6.8 or whatever you > have there: BIG NOPE. > > > > > Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> > > --- > > Subject: it never ends with full stop. > > A nit, subject: drop second/last, redundant "bindings". The > "dt-bindings" prefix is already stating that these are bindings. > See also: > https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > > + items: > > + - const: apb_pclk > > Drop _pclk > > > + > > + reg-names: > > Please follow DTS coding style about order of properties. > > > + items: > > + - const: ccu-base > > + > > + in-ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > + > > + unevaluatedProperties: > > This was never tested and it cannot reliably work. > > Sorry, this is waste of our time. > > Sorry about that. I will re-check the dt-binding file. > > + patternProperties: > > + '^port(@[0-7])?$': > > + description: Input connections from CoreSight Trace bus > > + $ref: /schemas/graph.yaml#/properties/port > > + > > + properties: > > + qcom,ccu-atid-offset: > > + description: > > + Offset to the Coresight Control Unit component's ATID register > > + that is used by specific TMC ETR. The ATID register can be programed based > > + on the trace id to filter out specific trace data which gets into ETR buffer. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + > > +required: > > + - compatible > > + - reg > > + - in-ports > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + syscon@1001000 { > > That's not a syscon. > > > + compatible = "qcom,coresight-ccu"; > > + reg = <0x1001000 0x1000>; > > + reg-names = "ccu-base"; > > + > > Best regards, > Krzysztof > Thanks, Jie
On 05/07/2024 10:00, Jie Gan wrote: > Add binding document for Coresight Control Unit device. nit: This is again too generic ? corsight-tmc-control-unit ? After all thats what it is and not a *generic* coresight control unit ? > > Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> > --- > .../bindings/arm/qcom,coresight-ccu.yaml | 87 +++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > > diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > new file mode 100644 > index 000000000000..9bb8ced393a7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > @@ -0,0 +1,87 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/arm/qcom,coresight-ccu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: CoreSight Control Unit > + > +maintainers: > + - Yuanfang Zhang <quic_yuanfang@quicinc.com> > + - Mao Jinlong <quic_jinlmao@quicinc.com> > + - Jie Gan <quic_jiegan@quicinc.com> > + > +description: > + The Coresight Control unit controls various Coresight behaviors. > + Used to enable/disable ETR’s data filter function based on trace ID. > + > +properties: > + compatible: > + const: qcom,coresight-ccu > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: apb_pclk > + > + reg-names: > + items: > + - const: ccu-base > + > + in-ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + unevaluatedProperties: > + patternProperties: > + '^port(@[0-7])?$': > + description: Input connections from CoreSight Trace bus > + $ref: /schemas/graph.yaml#/properties/port > + > + properties: > + qcom,ccu-atid-offset: Why do we need this atid offset ? Couldn't this be mapped to the "port" number ? e.g, input-port 0 on CCU => Offset x input-port 1 on CCU => (Offset x + Size of 1 region) I believe I mentioned this in the previous posting too ? Suzuki
On Mon, Jul 08, 2024 at 10:41:55AM +0100, Suzuki K Poulose wrote: > On 05/07/2024 10:00, Jie Gan wrote: > > Add binding document for Coresight Control Unit device. > > nit: This is again too generic ? corsight-tmc-control-unit ? After all > thats what it is and not a *generic* coresight control unit ? > coresight-tmc-control-unit is much better. We will check it. > > > > Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> > > --- > > .../bindings/arm/qcom,coresight-ccu.yaml | 87 +++++++++++++++++++ > > 1 file changed, 87 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > > > > diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > > new file mode 100644 > > index 000000000000..9bb8ced393a7 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > > @@ -0,0 +1,87 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/arm/qcom,coresight-ccu.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: CoreSight Control Unit > > + > > +maintainers: > > + - Yuanfang Zhang <quic_yuanfang@quicinc.com> > > + - Mao Jinlong <quic_jinlmao@quicinc.com> > > + - Jie Gan <quic_jiegan@quicinc.com> > > + > > +description: > > + The Coresight Control unit controls various Coresight behaviors. > > + Used to enable/disable ETR’s data filter function based on trace ID. > > + > > +properties: > > + compatible: > > + const: qcom,coresight-ccu > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + clock-names: > > + items: > > + - const: apb_pclk > > + > > + reg-names: > > + items: > > + - const: ccu-base > > + > > + in-ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > + > > + unevaluatedProperties: > > + patternProperties: > > + '^port(@[0-7])?$': > > + description: Input connections from CoreSight Trace bus > > + $ref: /schemas/graph.yaml#/properties/port > > + > > + properties: > > + qcom,ccu-atid-offset: > > Why do we need this atid offset ? Couldn't this be mapped to the "port" > number ? > > e.g, input-port 0 on CCU => Offset x > input-port 1 on CCU => (Offset x + Size of 1 region) If the first ATID offset remains constant, it appears to be feasible. We will consider the possibility of this solution. > > I believe I mentioned this in the previous posting too ? Yes, you mentioned before. I moved it from TMC filed to CCU filed. > > Suzuki >
On Mon, Jul 08, 2024 at 06:10:28PM +0800, JieGan wrote: > On Mon, Jul 08, 2024 at 10:41:55AM +0100, Suzuki K Poulose wrote: > > On 05/07/2024 10:00, Jie Gan wrote: > > > Add binding document for Coresight Control Unit device. > > > > nit: This is again too generic ? corsight-tmc-control-unit ? After all > > thats what it is and not a *generic* coresight control unit ? > > > coresight-tmc-control-unit is much better. We will check it. > > > > > > > Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> > > > --- > > > .../bindings/arm/qcom,coresight-ccu.yaml | 87 +++++++++++++++++++ > > > 1 file changed, 87 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > > > new file mode 100644 > > > index 000000000000..9bb8ced393a7 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml > > > @@ -0,0 +1,87 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/arm/qcom,coresight-ccu.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: CoreSight Control Unit > > > + > > > +maintainers: > > > + - Yuanfang Zhang <quic_yuanfang@quicinc.com> > > > + - Mao Jinlong <quic_jinlmao@quicinc.com> > > > + - Jie Gan <quic_jiegan@quicinc.com> > > > + > > > +description: > > > + The Coresight Control unit controls various Coresight behaviors. > > > + Used to enable/disable ETR’s data filter function based on trace ID. > > > + > > > +properties: > > > + compatible: > > > + const: qcom,coresight-ccu > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + clocks: > > > + maxItems: 1 > > > + > > > + clock-names: > > > + items: > > > + - const: apb_pclk > > > + > > > + reg-names: > > > + items: > > > + - const: ccu-base > > > + > > > + in-ports: > > > + $ref: /schemas/graph.yaml#/properties/ports > > > + > > > + unevaluatedProperties: > > > + patternProperties: > > > + '^port(@[0-7])?$': > > > + description: Input connections from CoreSight Trace bus > > > + $ref: /schemas/graph.yaml#/properties/port > > > + > > > + properties: > > > + qcom,ccu-atid-offset: > > > > Why do we need this atid offset ? Couldn't this be mapped to the "port" > > number ? > > > > e.g, input-port 0 on CCU => Offset x > > input-port 1 on CCU => (Offset x + Size of 1 region) > If the first ATID offset remains constant, it appears to be feasible. > We will consider the possibility of this solution. We just checked the ATID offset varies across different hardware platforms. It defined as 0xf4 on some platforms, and some others defined as 0xf8. So I think it should be better to define it in device tree node. > > > > > I believe I mentioned this in the previous posting too ? > Yes, you mentioned before. I moved it from TMC filed to CCU filed. > > > > > Suzuki > > >
diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml new file mode 100644 index 000000000000..9bb8ced393a7 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml @@ -0,0 +1,87 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/arm/qcom,coresight-ccu.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: CoreSight Control Unit + +maintainers: + - Yuanfang Zhang <quic_yuanfang@quicinc.com> + - Mao Jinlong <quic_jinlmao@quicinc.com> + - Jie Gan <quic_jiegan@quicinc.com> + +description: + The Coresight Control unit controls various Coresight behaviors. + Used to enable/disable ETR’s data filter function based on trace ID. + +properties: + compatible: + const: qcom,coresight-ccu + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + clock-names: + items: + - const: apb_pclk + + reg-names: + items: + - const: ccu-base + + in-ports: + $ref: /schemas/graph.yaml#/properties/ports + + unevaluatedProperties: + patternProperties: + '^port(@[0-7])?$': + description: Input connections from CoreSight Trace bus + $ref: /schemas/graph.yaml#/properties/port + + properties: + qcom,ccu-atid-offset: + description: + Offset to the Coresight Control Unit component's ATID register + that is used by specific TMC ETR. The ATID register can be programed based + on the trace id to filter out specific trace data which gets into ETR buffer. + $ref: /schemas/types.yaml#/definitions/uint32 + +required: + - compatible + - reg + - in-ports + +additionalProperties: false + +examples: + - | + syscon@1001000 { + compatible = "qcom,coresight-ccu"; + reg = <0x1001000 0x1000>; + reg-names = "ccu-base"; + + in-ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + ccu_in_port0: endpoint { + qcom,ccu-atid-offset = <0x1f>; + remote-endpoint = <&etr0_out_port>; + }; + }; + + port@1 { + reg = <1>; + ccu_in_port1: endpoint { + qcom,ccu-atid-offset = <0x2f>; + remote-endpoint = <&etr1_out_port>; + }; + }; + }; + };
Add binding document for Coresight Control Unit device. Signed-off-by: Jie Gan <quic_jiegan@quicinc.com> --- .../bindings/arm/qcom,coresight-ccu.yaml | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ccu.yaml