Re: [PATCH v4 2/4] dt-bindings: clock: Add YAML schemas for LPASS clocks on SC7180

From: Stephen Boyd <sboyd@kernel.org>
To: Michael Turquette <mturquette@baylibre.com>,
	Taniya Das <tdas@codeaurora.org>
Cc: David Brown <david.brown@linaro.org>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org,
	linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
	Andy Gross <agross@kernel.org>,
	devicetree@vger.kernel.org, robh@kernel.org, robh+dt@kernel.org,
	Taniya Das <tdas@codeaurora.org>
Subject: Re: [PATCH v4 2/4] dt-bindings: clock: Add YAML schemas for LPASS clocks on SC7180
Date: Tue, 21 Jul 2020 00:51:56 -0700	[thread overview]
Message-ID: <159531791606.3847286.11896325263133276429@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <1594795010-9074-3-git-send-email-tdas@codeaurora.org>

Quoting Taniya Das (2020-07-14 23:36:48)
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7180-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7180-lpasscorecc.yaml
> new file mode 100644
> index 0000000..41d0a6d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,sc7180-lpasscorecc.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,sc7180-lpasscorecc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm LPASS Core Clock Controller Binding for SC7180
> +
> +maintainers:
> +  - Taniya Das <tdas@codeaurora.org>
> +
> +description: |
> +  Qualcomm LPASS core clock control module which supports the clocks and
> +  power domains on SC7180.
> +
> +  See also:
> +  - dt-bindings/clock/qcom,lpasscorecc-sc7180.h
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sc7180-lpasshm
> +      - qcom,sc7180-lpasscorecc
> +
> +  clocks:
> +    items:
> +      - description: gcc_lpass_sway clock from GCC

Needs bi_tcxo description.

> +
> +  clock-names:
> +    items:
> +      - const: iface

Needs bi_tcxo

> +
> +  power-domains:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  '#power-domain-cells':
> +    const: 1
> +
> +  reg:
> +    minItems: 1
> +    items:
> +      - description: lpass core cc register
> +      - description: lpass audio cc register
> +
> +  reg-names:
> +    items:
> +      - const: lpass_core_cc
> +      - const: lpass_audio_cc
> +
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: qcom,sc7180-lpasshm
> +then:
> +  properties:
> +    reg:
> +      maxItems: 1
> +
> +else:
> +  properties:
> +    reg:
> +      minItems: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#power-domain-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
> +    #include <dt-bindings/clock/qcom,lpasscorecc-sc7180.h>
> +    clock-controller@63000000 {
> +      compatible = "qcom,sc7180-lpasshm";
> +        reg = <0x63000000 0x28>;
> +        clocks = <&gcc GCC_LPASS_CFG_NOC_SWAY_CLK>;
> +        clock-names = "iface";
> +        #clock-cells = <1>;
> +        #power-domain-cells = <1>;
> +    };
> +
> +  - |
> +    clock-controller@62d00000 {
> +        compatible = "qcom,sc7180-lpasscorecc";
> +        reg = <0x62d00000 0x50000>,
> +            <0x62780000 0x30000>;

Tabbing looks wrong. Just use spaces?

> +        reg-names = "lpass_core_cc", "lpass_audio_cc";
> +        clocks = <&gcc GCC_LPASS_CFG_NOC_SWAY_CLK>;
> +        clock-names = "iface";
> +        power-domains = <&lpass_hm LPASS_CORE_HM_GDSCR>;
> +        #clock-cells = <1>;
> +        #power-domain-cells = <1>;
> +    };
> +...
> diff --git a/include/dt-bindings/clock/qcom,gcc-sc7180.h b/include/dt-bindings/clock/qcom,gcc-sc7180.h
> index 992b67b..bdf43adc 100644
> --- a/include/dt-bindings/clock/qcom,gcc-sc7180.h
> +++ b/include/dt-bindings/clock/qcom,gcc-sc7180.h
> @@ -138,6 +138,7 @@
>  #define GCC_MSS_Q6_MEMNOC_AXI_CLK                              128
>  #define GCC_MSS_SNOC_AXI_CLK                                   129
>  #define GCC_SEC_CTRL_CLK_SRC                                   130
> +#define GCC_LPASS_CFG_NOC_SWAY_CLK                             131
>  
>  /* GCC resets */
>  #define GCC_QUSB2PHY_PRIM_BCR                                  0

This hunk should be in the next patch. Oh but then that patch should come
before this one so the binding can use it. Either way, shouldn't be part
of this patch.