Re: [PATCH 6/9] dt-bindings: remoteproc: qcom: mss: Convert bindings to YAML

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux