Re: [PATCH v2 2/4] dt-bindings: arm: Add binding document for Coresight Control Unit device.

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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