| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <80417bba-2ddb-2730-c9fd-6879d38bf126@linaro.org>
Date: Thu, 24 Nov 2022 13:05:16 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Abel Vesa <abel.vesa@...aro.org>, Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Georgi Djakov <djakov@...nel.org>,
Odelu Kukatla <okukatla@...eaurora.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc: Melody Olvera <quic_molvera@...cinc.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-pm@...r.kernel.org
Subject: Re: [PATCH v2 1/3] dt-bindings: interconnect: Add schema for SM8550
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@...aro.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@...nel.org>
> + - Odelu Kukatla <okukatla@...eaurora.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@...0000 {
> + compatible = "qcom,sm8550-cnoc-main";
> + reg = <0x01500000 0x13080>;
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
> + aggre1_noc: interconnect@...0000 {
> + 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@...0000 {
> + 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
Powered by blists - more mailing lists