[v2,1/3] dt-bindings: interconnect: Add schema for SM8550

Add dedicated schema file for SM8500. This allows better constraining
of reg property, depending on the type of the NOC node. Also allows
better constraining of the clocks property. All of the above while
keeping the file itself comprehensible.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 .../interconnect/qcom,sm8550-rpmh.yaml        | 141 ++++++++++++++++++
 1 file changed, 141 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sm8550-rpmh.yaml

On 24/11/2022 12:22, Abel Vesa wrote:
> Add dedicated schema file for SM8500. This allows better constraining
> of reg property, depending on the type of the NOC node. Also allows
> better constraining of the clocks property. All of the above while
> keeping the file itself comprehensible.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  .../interconnect/qcom,sm8550-rpmh.yaml        | 141 ++++++++++++++++++
>  1 file changed, 141 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sm8550-rpmh.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sm8550-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sm8550-rpmh.yaml
> new file mode 100644
> index 000000000000..9627b629d4ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,sm8550-rpmh.yaml
> @@ -0,0 +1,141 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/qcom,sm8550-rpmh.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm RPMh Network-On-Chip Interconnect on SM8550
> +
> +maintainers:
> +  - Georgi Djakov <djakov@kernel.org>
> +  - Odelu Kukatla <okukatla@codeaurora.org>

I think this is not accurate email. Georgi also might not be interested
in SM8550 itself, so I propose add here yourself and Neil.

> +
> +description: |
> +   RPMh interconnect providers support system bandwidth requirements through
> +   RPMh hardware accelerators known as Bus Clock Manager (BCM). The provider is
> +   able to communicate with the BCM through the Resource State Coordinator (RSC)
> +   associated with each execution environment. Provider nodes must point to at
> +   least one RPMh device child node pertaining to their RSC and each provider
> +   can map to multiple RPMh resources.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sm8550-aggre1-noc
> +      - qcom,sm8550-aggre2-noc
> +      - qcom,sm8550-clk-virt
> +      - qcom,sm8550-cnoc-main
> +      - qcom,sm8550-config-noc
> +      - qcom,sm8550-gem-noc
> +      - qcom,sm8550-lpass-ag-noc
> +      - qcom,sm8550-lpass-lpiaon-noc
> +      - qcom,sm8550-lpass-lpicx-noc
> +      - qcom,sm8550-mc-virt
> +      - qcom,sm8550-mmss-noc
> +      - qcom,sm8550-nsp-noc
> +      - qcom,sm8550-pcie-anoc
> +      - qcom,sm8550-system-noc
> +

reg:
  maxItems: 1

> +allOf:
> +  - $ref: qcom,rpmh-common.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8550-aggre1-noc
> +              - qcom,sm8550-aggre2-noc
> +              - qcom,sm8550-cnoc-main
> +              - qcom,sm8550-config-noc
> +              - qcom,sm8550-gem-noc
> +              - qcom,sm8550-lpass-ag-noc
> +              - qcom,sm8550-lpass-lpiaon-noc
> +              - qcom,sm8550-lpass-lpicx-noc
> +              - qcom,sm8550-mmss-noc
> +              - qcom,sm8550-nsp-noc
> +              - qcom,sm8550-pcie-anoc
> +              - qcom,sm8550-system-noc
> +    then:
> +      properties:
> +        reg:
> +          minItems: 1
> +          maxItems: 1

Instead:

  if:
     ....

     enum:
       - virt interconnects
  then:
    properties:
      reg: false
  else:
    required:
      - reg


> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8550-pcie-anoc
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: aggre-NOC PCIe AXI clock
> +            - description: cfg-NOC PCIe a-NOC AHB clock
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8550-aggre1-noc
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: aggre UFS PHY AXI clock
> +            - description: aggre USB3 PRIM AXI clock
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8550-aggre2-noc
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: RPMH CC IPA clock

This part is in general fine, but I propose to do it differently -
mentioning clocks in top-level properties, because it's defining
properties in allOf:if:then is error prone and easy to miss. Better to
have one definition and customization in if:then:.

Therefore:
1. in top-level properties:
  clocks:
    minItems: 1
    maxItems: 2

2. All your ifs stay the same.

3. One more if: compatible: enum: pcie/aggre1/aggre2

  then:
    required:
      - clocks
  else:
    properties:
      clocks: false


> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +      #include <dt-bindings/clock/qcom,gcc-sm8550.h>
> +      #include <dt-bindings/interconnect/qcom,sm8550.h>
> +      #include <dt-bindings/clock/qcom,rpmh.h>
> +
> +      clk_virt: interconnect-0 {
> +             compatible = "qcom,sm8550-clk-virt";
> +             #interconnect-cells = <2>;
> +             qcom,bcm-voters = <&apps_bcm_voter>;
> +      };
> +
> +      cnoc_main: interconnect@1500000 {
> +             compatible = "qcom,sm8550-cnoc-main";
> +             reg = <0x01500000 0x13080>;
> +             #interconnect-cells = <2>;
> +             qcom,bcm-voters = <&apps_bcm_voter>;
> +      };
> +
> +      aggre1_noc: interconnect@16e0000 {
> +             compatible = "qcom,sm8550-aggre1-noc";
> +             reg = <0x016e0000 0x14400>;
> +             #interconnect-cells = <2>;
> +             clocks = <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
> +                      <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>;
> +             qcom,bcm-voters = <&apps_bcm_voter>;
> +      };
> +
> +      aggre2_noc: interconnect@1700000 {
> +             compatible = "qcom,sm8550-aggre2-noc";
> +             reg = <0x01700000 0x1E400>;
> +             #interconnect-cells = <2>;
> +             clocks = <&rpmhcc RPMH_IPA_CLK>;
> +             qcom,bcm-voters = <&apps_bcm_voter>;
> +      };

and keep just two examples (e.g. virt and aggre1) - they differ only by
one or two properties, so it's a bit too much...

Best regards,
Krzysztof

On Thu, 24 Nov 2022 13:22:30 +0200, Abel Vesa wrote:
> Add dedicated schema file for SM8500. This allows better constraining
> of reg property, depending on the type of the NOC node. Also allows
> better constraining of the clocks property. All of the above while
> keeping the file itself comprehensible.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  .../interconnect/qcom,sm8550-rpmh.yaml        | 141 ++++++++++++++++++
>  1 file changed, 141 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sm8550-rpmh.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/interconnect/qcom,sm8550-rpmh.example.dts:18:18: fatal error: dt-bindings/clock/qcom,gcc-sm8550.h: No such file or directory
   18 |         #include <dt-bindings/clock/qcom,gcc-sm8550.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/interconnect/qcom,sm8550-rpmh.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221124112232.1704144-2-abel.vesa@linaro.org

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.