On 11/05/2022 18:15, Sireesh Kodali wrote: > Convert the bindings to YAML from txt. The bindings follow a similar > schema to `qcom,adsp.yaml`. > > Signed-off-by: Sireesh Kodali <sireeshkodali1@xxxxxxxxx> > --- > .../bindings/remoteproc/qcom,q6v5.txt | 302 -------- > .../bindings/remoteproc/qcom,q6v5.yaml | 702 ++++++++++++++++++ > 2 files changed, 702 insertions(+), 302 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt > create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,q6v5.yaml Same comments as for patch 4 apply here. Some of them seems fixed so it makes me wonder - why you wrote two bindings in entirely different way? > + This document defines the binding for a component that loads and boots firmware > + on the Qualcomm Hexagon core. > + > +properties: > + compatible: > + # Special case, because older platforms like MSM8916 use both compatibles together > + minItems: 1 > + maxItems: 2 > + oneOf: > + - items: > + - enum: > + - qcom,msm8916-mss-pil > + - enum: > + - qcom,q6v5-pil Everywhere - same comments as in patch 4. > + - items: > + - enum: > + - qcom,q6v5-pil > + - qcom,ipq8074-wcss-pil > + - qcom,qcs404-wcss-pil > + - qcom,msm8916-mss-pil > + - qcom,msm8974-mss-pil > + - qcom,msm8996-mss-pil > + - qcom,msm8998-mss-pil > + - qcom,sc7180-mss-pil > + - qcom,sc7280-mss-pil > + - qcom,sdm845-mss-pil > + > + reg: > + description: must specify the base address and size of the qdsp6 and rmb > + register blocks > + maxItems: 2 > + > + reg-names: > + items: > + - const: qdsp6 > + - const: rmb > + > + interrupts-extended: > + minItems: 5 > + maxItems: 6 > + > + interrupt-names: > + minItems: 5 > + maxItems: 6 > + > + firmware-name: > + $ref: /schemas/types.yaml#/definitions/string-array > + description: List the relative firmware image paths for the mba and > + modem. They are used for booting and authenticating the Hexagon core. > + maxItems: 2 > + > + clocks: > + minItems: 4 > + maxItems: 10 > + > + clock-names: > + minItems: 4 > + maxItems: 10 > + > + resets: > + description: Reference to the reset-controllwer for the modem subsystem > + minItems: 1 > + maxItems: 3 > + > + reset-names: > + minItems: 1 > + maxItems: 3 > + > + memory-region: > + maxItems: 1 > + description: Reference to the reserved-memory for the mba region followed > + by the mpss region. Required if the mba and mpss sub-nodes are not > + specified. > + > + cx-supply: > + description: Phandle to the CX regulator > + > + mx-supply: > + description: Phandle to the MX regulator > + > + pll-supply: > + description: Phandle to the PLL regulator, to be held on behalf of the > + booting Hexagon core > + > + mss-supply: > + description: Phandle to the mss regulator, to be held on behalf of the > + booting Hexagon core s/Phandle to//, everywhere. > + > + power-domains: > + minItems: 1 > + maxItems: 4 > + > + power-domain-names: > + minItems: 1 > + maxItems: 4 > + > + qcom,qmp: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: Reference to the AOSS side-channel message RAM. > + > + qcom,smem-states: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: States used by the AP to signal the Hexagon core > + items: > + - description: Stop the modem > + > + qcom,smem-state-names: > + $ref: /schemas/types.yaml#/definitions/string-array > + description: The names of the state bits used for SMP2P output > + items: > + - const: stop > + > + qcom,halt-regs: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + Phandle reference to a syscon representing TCSR followed by the > + three offsets within syscon for q6, modem and nc halt registers. > + > + qcom,spare-regs: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + A phandle reference to a syscon representing TCSR followed by the > + offset within syscon for conn_box_spare0 register used by the modem > + sub-system running on SC7180 SoC. > + > + qcom,ext-regs: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + Two phandle references to syscons representing TCSR_REG and TCSR > + register space followed by the two offsets within the syscon to > + force_clk_en/rscc_disable and axim1_clk_off/crypto_clk_off registers > + respectively. > + > + qcom,qaccept-regs: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + A phandle reference to a syscon representing TCSR followed by the three > + offsets within syscon for mdm, cx and axi qaccept registers used by the > + modem sub-system running on SC7280 SoC. > + > + iommus: > + description: > + Only required on platforms that do not have TrustZone. > + > + smd-edge: > + type: object > + description: > + Qualcomm Shared Memory subnode which represents communication edge, > + channels and devices related to the ADSP. > + > + glink-edge: > + type: object > + description: > + Qualcomm G-Link subnode which represents communication edge, channels > + and devices related to the ADSP. > + > + mba: > + type: object description needed. > + properties: > + memory-region: > + maxItems: 1 > + > + required: > + - memory-region > + > + mpss: description needed. > + type: object > + properties: > + memory-region: > + maxItems: 1 > + > + required: > + - memory-region > + > +required: > + - compatible > + - clocks > + - clock-names > + - interrupts-extended > + - interrupt-names > + - qcom,smem-states > + - qcom,smem-state-names > + > +additionalProperties: false > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,ipq8074-wcss-pil > + then: > + properties: > + clocks: false > + clock-names: false > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,qcs404-wcss-pil > + then: > + properties: > + clocks: > + items: > + - description: GCC_AHBS_CBCR clock > + - description: GCC_AXIM_CBCR clock > + - description: LCC_AHBFABRIC_CBC clock > + - description: TCSR_LCC_CBC clock > + - description: LCC_AHBS_CBC clock > + - description: LCC_TCM_SLAVE_CBC clock > + - description: LCC_ABHM_CBC clock > + - description: LCC_AXIM_CBC clock > + - description: LCC_BCR_SLEEP clock > + - description: XO clock > + clock-names: > + items: > + - const: gcc_ahbs_cbcr > + - const: gcc_axim_cbcr > + - const: lcc_ahbfabric_cbc > + - const: tcsr_lcc_cbc > + - const: lcc_abhs_cbc > + - const: lcc_tcm_slave_cbc > + - const: lcc_abhm_cbc > + - const: lcc_axim_cbc > + - const: lcc_bcr_sleep > + - const: xo > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,q6v5-pil > + - qcom,msm8916-mss-pil > + - qcom,msm8974-mss-pil > + then: > + properties: > + clocks: > + items: > + - description: Interface clock > + - description: Bus clock > + - description: Memory clock > + - description: XO clock > + clock-names: > + items: > + - const: iface > + - const: bus > + - const: mem > + - const: xo > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,msm8996-mss-pil > + then: > + properties: > + clocks: > + items: > + - description: Interface clock > + - description: Bus clock > + - description: Memory clock > + - description: GPLL0_MSS clock > + - description: SNOC_AXI clock > + - description: MNOC_AXI clock > + - description: PNOC clock > + - description: QDSS clock > + - description: XO clock > + clock-names: > + items: > + - const: iface > + - const: bus > + - const: mem > + - const: gpll0_mss > + - const: snoc_axi > + - const: mnoc_axi > + - const: pnoc > + - const: qdss > + - const: xo > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,msm8998-mss-pil > + then: > + properties: > + clocks: > + items: > + - description: Interface clock > + - description: Bus clock > + - description: Memory clock > + - description: GPLL0_MSS clock > + - description: SNOC_AXI clock > + - description: MNOC_AXI clock > + - description: QDSS clock > + - description: XO clock > + clock-names: > + items: > + - const: iface > + - const: bus > + - const: mem > + - const: gpll0_mss > + - const: snoc_axi > + - const: mnoc_axi > + - const: qdss > + - const: xo > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,sc7180-mss-pil > + then: > + properties: > + clocks: > + items: > + - description: Interface clock > + - description: Bus clock > + - description: NAV clock > + - description: SNOC_AXI clock > + - description: MNOC_AXI clock > + - description: XO clock > + clock-names: > + items: > + - const: iface > + - const: bus > + - const: nav > + - const: snoc_axi > + - const: mnoc_axi > + - const: xo > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,sc7280-mss-pil > + then: > + properties: > + clocks: > + items: > + - description: Interface clock > + - description: Offline clock > + - description: SNOC_AXI clock > + - description: PKA clock > + - description: XO clock > + clock-names: > + items: > + - const: iface > + - const: offline > + - const: snoc_axi > + - const: pka > + - const: xo > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,sdm845-mss-pil > + then: > + properties: > + clocks: > + items: > + - description: Interface clock > + - description: Bus clock > + - description: Memory clock > + - description: GPLL0_MSS clock > + - description: SNOC_AXI clock > + - description: MNOC_AXI clock > + - description: PRNG clock > + - description: XO clock > + clock-names: > + items: > + - const: iface > + - const: bus > + - const: mem > + - const: gpll0_mss > + - const: snoc_axi > + - const: mnoc_axi > + - const: prng > + - const: xo > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,q6v5-pil > + - qcom,ipq8074-wcss-pil > + - qcom,qcs404-wcss-pil > + - qcom,msm8916-mss-pil > + - qcom,msm8974-mss-pil > + then: > + properties: > + interrupts-extended: No extended. > + items: > + - description: Watchdog interrupt > + - description: Fatal interrupt > + - description: Ready interrupt > + - description: Handover interrupt > + - description: Stop acknowledge interrupt > + interrupt-names: > + items: > + - const: wdog > + - const: fatal > + - const: ready > + - const: handover > + - const: stop-ack > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,msm8996-mss-pil > + - qcom,msm8998-mss-pil > + - qcom,sc7180-mss-pil > + - qcom,sc7280-mss-pil > + - qcom,sdm845-mss-pil > + then: > + properties: > + interrupts-extended: > + items: > + - description: Watchdog interrupt > + - description: Fatal interrupt > + - description: Ready interrupt > + - description: Handover interrupt > + - description: Stop acknowledge interrupt > + - description: Shutdown acknowledge interrupt > + interrupt-names: > + items: > + - const: wdog > + - const: fatal > + - const: ready > + - const: handover > + - const: stop-ack > + - const: shutdown-ack Both ifs should be written differently. Move the bigger list to properties with minItems: 5. For qcom,q6v5-pil you set maxItems:5. For qcom,msm8996-mss-pil you set minItems:6. > + > + - if: > + properties: > + compatible: > + enum: > + - const: qcom,q6v5-pil > + - const: qcom,msm8916-mss-pil > + - const: qcom,msm8974-mss-pil > + then: > + properties: > + cx-supply: > + deprecated: true > + mx-supply: > + deprecated: true > + > + required: > + - pll-supply > + > + - if: > + properties: > + compatible: > + enum: > + - qcom,msm8916-mss-pil > + - qcom,msm8974-mss-pil > + - qcom,msm8996-mss-pil > + - qcom,msm8998-mss-pil > + then: > + properties: > + power-domains: > + items: > + - description: CX power domain > + - description: MX power domain > + power-domain-names: > + items: > + - const: cx > + - const: mx > + > + required: > + - power-domains > + - power-domain-names > + > + - if: > + properties: > + compatible: > + enum: > + - const: qcom,msm8996-mss-pil > + then: > + properties: > + cx-supply: false > + mx-supply: false > + > + required: > + - pll-supply > + > + - if: > + properties: > + compatible: > + enum: > + - const: qcom,qcs404-wcss-pil > + then: > + required: > + - cx-supply > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,sdm845-mss-pil > + - qcom,sc7180-mss-pil > + then: > + properties: > + power-domains: > + items: > + - description: CX power domain > + - description: MX power domain > + - description: MSS power domain > + power-domain-names: > + items: > + - const: cx > + - const: mx > + - const: mss > + > + required: > + - power-domains > + - power-domain-names > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,sc7280-mss-pil > + then: > + properties: > + power-domains: > + items: > + - description: CX power domain > + - description: MSS power domain > + power-domain-names: > + items: > + - const: cx > + - const: mss > + > + required: > + - power-domains > + - power-domain-names > + - qcom,ext-regs > + - qcom,qaccept-regs > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,q6v5-pil > + - qcom,msm8916-mss-pil > + - qcom,msm8974-mss-pil > + - qcom,msm8996-mss-pil > + - qcom,msm8998-mss-pil > + then: > + properties: > + resets: > + items: > + - description: MSS reset > + reset-names: > + items: > + - const: mss_restart > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,ipq8074-wcss-pil > + - qcom,qcs404-wcss-pil > + then: > + properties: > + resets: > + items: > + - description: WCSS Always On restart > + - description: WCSS reset > + - description: WCSS Q6 reset > + reset-names: > + items: > + - const: wcss_aon_restart > + - const: wcss_reset > + - const: wcss_q6_reset > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,sc7180-mss-pil > + - qcom,sc7280-mss-pil > + - qcom,sdm845-mss-pil > + then: > + properties: > + resets: > + items: > + - description: MSS restart > + - description: PDC reset > + reset-names: > + items: > + - const: mss_restart > + - const: pdc_reset > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,q6v5-pil > + - qcom,ipq8074-wcss-pil > + - qcom,qcs404-wcss-pil > + - qcom,msm8916-mss-pil > + - qcom,msm8974-mss-pil > + - qcom,msm8996-mss-pil > + - qcom,msm8998-mss-pil > + then: > + properties: > + qcom,qmp: false > + iommus: false > + > +examples: > + - | > + #include <dt-bindings/clock/qcom,rpmcc.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/clock/qcom,gcc-msm8974.h> > + #include <dt-bindings/reset/qcom,gcc-msm8974.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + modem-rproc@fc880000 { Generic node name, so "remoteproc" > + compatible = "qcom,q6v5-pil"; > + reg = <0xfc880000 0x100>, > + <0xfc820000 0x020>; > + reg-names = "qdsp6", "rmb"; > + > + interrupts-extended = <&intc GIC_SPI 24 IRQ_TYPE_EDGE_RISING>, > + <&modem_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, > + <&modem_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, > + <&modem_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, > + <&modem_smp2p_in 3 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "wdog", > + "fatal", > + "ready", > + "handover", > + "stop-ack"; > + > + clocks = <&gcc GCC_MSS_Q6_BIMC_AXI_CLK>, > + <&gcc GCC_MSS_CFG_AHB_CLK>, > + <&gcc GCC_BOOT_ROM_AHB_CLK>, > + <&xo_board>; > + clock-names = "iface", "bus", "mem", "xo"; > + > + qcom,halt-regs = <&tcsr_mutex_block 0x1180 0x1200 0x1280>; Does this pass the tests? Did you test the bindings? > + > + resets = <&gcc GCC_MSS_RESTART>; > + reset-names = "mss_restart"; > + > + cx-supply = <&pm8841_s2>; > + mss-supply = <&pm8841_s3>; > + mx-supply = <&pm8841_s1>; > + pll-supply = <&pm8941_l12>; > + > + qcom,smem-states = <&modem_smp2p_out 0>; > + qcom,smem-state-names = "stop"; > + > + mba { > + memory-region = <&mba_region>; Wrong indentation. > + }; > + > + mpss { > + memory-region = <&mpss_region>; > + }; > + > + smd-edge { > + interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>; > + > + qcom,ipc = <&apcs 8 12>; > + qcom,smd-edge = <0>; > + > + label = "modem"; > + }; > + }; Best regards, Krzysztof