[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8704fbd6-94a1-5692-5ccb-95f66ffaaf6f@ixit.cz>
Date: Sun, 26 Jun 2022 19:13:54 +0200
From: David Heidelberg <david@...t.cz>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: ~okias/devicetree@...ts.sr.ht, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Das Srinagesh <quic_gurus@...cinc.com>,
Robert Marko <robimarko@...il.com>
Subject: Re: [PATCH v3 3/3] dt-bindings: firmware: convert Qualcomm SCM
binding to the yaml
On 26/06/2022 18:45, Krzysztof Kozlowski wrote:
> On 26/06/2022 13:46, David Heidelberg wrote:
>> Convert Qualcomm SCM firmware binding to the yaml format.
>>
>> This commit also:
>> - adds qcom,scm-mdm9607 into list which has only core clock
>> - adds qcom,scm-sm6125, qcom,scm-ipq6018
>> - #reset-cells, because the property is already used
>>
>> Cc: Robert Marko <robimarko@...il.com>
>> Cc: Guru Das Srinagesh <quic_gurus@...cinc.com>
>> Signed-off-by: David Heidelberg <david@...t.cz>
>> ---
>> v3:
>> - add preceding patches for ARM and arm64 adding missing compatible strings
>> - extended with missing compatible strings
>> - added two additional maintainers, see https://lkml.org/lkml/2022/6/23/1969
>> v2:
>> - changed maintainer to Bjorn
>> - document #reset-cells
>>
>> .../devicetree/bindings/firmware/qcom,scm.txt | 57 --------
>> .../bindings/firmware/qcom,scm.yaml | 131 ++++++++++++++++++
>> 2 files changed, 131 insertions(+), 57 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/firmware/qcom,scm.txt
>> create mode 100644 Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
>> deleted file mode 100644
>> index 0f4e5ab26477..000000000000
>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.txt
>> +++ /dev/null
>> @@ -1,57 +0,0 @@
>> -QCOM Secure Channel Manager (SCM)
>> -
>> -Qualcomm processors include an interface to communicate to the secure firmware.
>> -This interface allows for clients to request different types of actions. These
>> -can include CPU power up/down, HDCP requests, loading of firmware, and other
>> -assorted actions.
>> -
>> -Required properties:
>> -- compatible: must contain one of the following:
>> - * "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-msm8976"
>> - * "qcom,scm-msm8994"
>> - * "qcom,scm-msm8996"
>> - * "qcom,scm-msm8998"
>> - * "qcom,scm-sc7180"
>> - * "qcom,scm-sc7280"
>> - * "qcom,scm-sdm845"
>> - * "qcom,scm-sdx55"
>> - * "qcom,scm-sm6350"
>> - * "qcom,scm-sm8150"
>> - * "qcom,scm-sm8250"
>> - * "qcom,scm-sm8350"
>> - * "qcom,scm-sm8450"
>> - and:
>> - * "qcom,scm"
>> -- clocks: Specifies clocks needed by the SCM interface, if any:
>> - * core clock required for "qcom,scm-apq8064", "qcom,scm-msm8660" and
>> - "qcom,scm-msm8960"
>> - * core, iface and bus clocks required for "qcom,scm-apq8084",
>> - "qcom,scm-msm8916", "qcom,scm-msm8953", "qcom,scm-msm8974" and "qcom,scm-msm8976"
>> -- clock-names: Must contain "core" for the core clock, "iface" for the interface
>> - clock and "bus" for the bus clock per the requirements of the compatible.
>> -- qcom,dload-mode: phandle to the TCSR hardware block and offset of the
>> - download mode control register (optional)
>> -
>> -Example for MSM8916:
>> -
>> - firmware {
>> - scm {
>> - compatible = "qcom,msm8916", "qcom,scm";
>> - clocks = <&gcc GCC_CRYPTO_CLK> ,
>> - <&gcc GCC_CRYPTO_AXI_CLK>,
>> - <&gcc GCC_CRYPTO_AHB_CLK>;
>> - clock-names = "core", "bus", "iface";
>> - };
>> - };
>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>> new file mode 100644
>> index 000000000000..17d06e75b82b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>> @@ -0,0 +1,131 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/firmware/qcom,scm.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> No quotes here.
>
>> +
>> +title: QCOM Secure Channel Manager (SCM)
>> +
>> +description: |
>> + Qualcomm processors include an interface to communicate to the secure firmware.
>> + This interface allows for clients to request different types of actions.
>> + These can include CPU power up/down, HDCP requests, loading of firmware,
>> + and other assorted actions.
>> +
>> +maintainers:
>> + - Bjorn Andersson <bjorn.andersson@...aro.org>
>> + - Robert Marko <robimarko@...il.com>
>> + - Guru Das Srinagesh <quic_gurus@...cinc.com>
>> +
>> +properties:
>> + compatible:
>> + items:
>> + - enum:
>> + - qcom,scm-apq8064
>> + - qcom,scm-apq8084
>> + - qcom,scm-ipq4019
>> + - qcom,scm-ipq6018
>> + - 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-msm8976
>> + - qcom,scm-msm8994
>> + - qcom,scm-msm8996
>> + - qcom,scm-msm8998
>> + - qcom,scm-sc7180
>> + - qcom,scm-sc7280
>> + - qcom,scm-sdm845
>> + - qcom,scm-sdx55
>> + - qcom,scm-sm6125
>> + - qcom,scm-sm6350
>> + - qcom,scm-sm8150
>> + - qcom,scm-sm8250
>> + - qcom,scm-sm8350
>> + - qcom,scm-sm8450
>> + - qcom,scm-qcs404
>> + - const: qcom,scm
>> +
>> + clocks:
>> + minItems: 1
>> + maxItems: 3
>> +
>> + clock-names: true
> You should have constraints here - min/maxItems.
>
>> +
>> + '#reset-cells':
>> + const: 1
>> +
>> + qcom,dload-mode:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + items:
>> + - items:
>> + - description: phandle to TCSR hardware block
>> + - description: offset of the download mode control register
>> + description:
>> + Should be phandle/offset pair.
> This description is not helpful. Should be something closer to "TCSR
> hardware block".
>
>> +
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,scm-apq8064
>> + - qcom,scm-mdm9607
>> + - qcom,scm-msm8660
>> + - qcom,scm-msm8960
>> + then:
>> + properties:
>> + clock-names:
>> + items:
>> + - const: core
> Missing constraints (maxItems:2) for clocks.
Why 2? I would put `maxItems: 1` there
>
>> +
>> + required:
>> + - clocks
>> + - clock-names
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,scm-apq8084
>> + - qcom,scm-msm8916
>> + - qcom,scm-msm8953
>> + - qcom,scm-msm8974
>> + - qcom,scm-msm8976
>> + then:
>> + properties:
>> + clock-names:
>> + items:
>> + - const: core
>> + - const: bus
>> + - const: iface
> Missing constraints for clocks.
>
>> +
>> + required:
>> + - clocks
>> + - clock-names
>> +
>> +required:
>> + - compatible
>> +
>> +additionalProperties: false
>> +
> Best regards,
> Krzysztof
--
David Heidelberg
Consultant Software Engineer
Matrix: @okias:matrix.org
Powered by blists - more mailing lists