Re: [PATCH] dt-bindings: firmware: convert Qualcomm SCM binding to the yaml

[Krzysztof Kozlowski]

On 18/12/2021 20:40, David Heidelberg wrote:
> Convert Qualcomm SCM firmware binding to the yaml format.
> 
> Signed-off-by: David Heidelberg <david@xxxxxxx>
> ---
> This patch comes with followup question -> since not all definitions
> follow `"qcom,scm-*chipset*", "qcom,scm"`, should I change them or adjust this
> binding to cover all cases?
> 

Thank you for your patch. I hope you will continue to work on this and
send a v2. :)

There is something to discuss/improve.

>  .../devicetree/bindings/firmware/qcom,scm.txt |  54 ---------
>  .../bindings/firmware/qcom,scm.yaml           | 112 ++++++++++++++++++
>  2 files changed, 112 insertions(+), 54 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/firmware/qcom,scm.txt
>  create mode 100644 Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> 

(...)

> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - qcom,scm-apq8064
> +          - qcom,scm-apq8084
> +          - qcom,scm-ipq4019
> +          - qcom,scm-ipq806x
> +          - qcom,scm-ipq8074
> +          - qcom,scm-mdm9607
> +          - qcom,scm-msm8226
> +          - qcom,scm-msm8660
> +          - qcom,scm-msm8916
> +          - qcom,scm-msm8953
> +          - qcom,scm-msm8960
> +          - qcom,scm-msm8974
> +          - qcom,scm-msm8994
> +          - qcom,scm-msm8996
> +          - qcom,scm-msm8998
> +          - qcom,scm-sc7180
> +          - qcom,scm-sc7280
> +          - qcom,scm-sdm845
> +          - qcom,scm-sdx55
> +          - qcom,scm-sm8150
> +          - qcom,scm-sm8250
> +          - qcom,scm-sm8350
> +      - const: qcom,scm
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 3
> +
> +  clock-names: true
> +
> +  qcom,dload-mode:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: >

No need for >

> +      TCSR hardware block and offset of the download mode control register

Could you define the items (and I think it has to be phandle-array in
such case) like here for samsung,sysreg:
https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42

This helps to validate the actual phandle.

The DTSes have also few other properties (like reset-cells). They can be
added in this commit, just please mention it in the commit msg.

> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,scm-apq8064
> +              - qcom,scm-msm8660
> +              - qcom,scm-msm8960
> +    then:
> +      properties:
> +        clock-names:
> +          items:
> +            - const: core
> +
> +      required:
> +        - clocks
> +        - clock-names
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,scm-apq8084

Based on the driver you also need (this can be in separate commit or
just mention in commit msg):
qcom,scm-mdm9607


Best regards,
Krzysztof