Re: [PATCH v9 1/4] dt-bindings: clock: Document external clocks for MSM8998 gcc

[Jeffrey Hugo]

On 11/13/2019 4:20 AM, Taniya Das wrote:
> Hi Jeffrey,
>
> On 11/13/2019 2:14 AM, Jeffrey Hugo wrote:
> > The global clock controller on MSM8998 can consume a number of external
> > clocks. Document them.
> >
> > For 7180 and 8150, the hardware always exists, so no clocks are truly
> > optional. Therefore, simplify the binding by removing the min/max
> > qualifiers to clocks. Also, fixup an example so that dt_binding_check
> > passes.
> >
> > Signed-off-by: Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx>
> > ---
> > Â .../devicetree/bindings/clock/qcom,gcc.yamlÂÂÂÂÂÂÂ | 47 
> > +++++++++++++++-------
> > Â 1 file changed, 33 insertions(+), 14 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc.yaml 
> > b/Documentation/devicetree/bindings/clock/qcom,gcc.yaml
> > index e73a56f..2f3512b 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,gcc.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,gcc.yaml
> > @@ -40,20 +40,38 @@ properties:
> > ÂÂÂÂÂÂÂÂ - qcom,gcc-sm8150
> > ÂÂÂ clocks:
> > -ÂÂÂ minItems: 1
> > -ÂÂÂ maxItems: 3
> > -ÂÂÂ items:
> > -ÂÂÂÂÂ - description: Board XO source
> > -ÂÂÂÂÂ - description: Board active XO source
> > -ÂÂÂÂÂ - description: Sleep clock source
> > +ÂÂÂ oneOf:
> > +ÂÂÂÂÂ #qcom,gcc-sm8150
> > +ÂÂÂÂÂ #qcom,gcc-sc7180
> > +ÂÂÂÂÂ - items:
> > +ÂÂÂÂÂÂÂ - description: Board XO source
> > +ÂÂÂÂÂÂÂ - description: Board active XO source
> > +ÂÂÂÂÂÂÂ - description: Sleep clock source
> > +ÂÂÂÂÂ #qcom,gcc-msm8998
> > +ÂÂÂÂÂ - items:
> > +ÂÂÂÂÂÂÂ - description: Board XO source
> > +ÂÂÂÂÂÂÂ - description: USB 3.0 phy pipe clock
> > +ÂÂÂÂÂÂÂ - description: UFS phy rx symbol clock for pipe 0
> > +ÂÂÂÂÂÂÂ - description: UFS phy rx symbol clock for pipe 1
> > +ÂÂÂÂÂÂÂ - description: UFS phy tx symbol clock
> > +ÂÂÂÂÂÂÂ - description: PCIE phy pipe clock
>
> Would it be possible to add an example for MSM8998?

It doesn't seem to be materially different that the existing examples, 
but sure, that's something that can be done.

> > ÂÂÂ clock-names:
> > -ÂÂÂ minItems: 1
> > -ÂÂÂ maxItems: 3
> > -ÂÂÂ items:
> > -ÂÂÂÂÂ - const: bi_tcxo
> > -ÂÂÂÂÂ - const: bi_tcxo_ao
> > -ÂÂÂÂÂ - const: sleep_clk
> > +ÂÂÂ oneOf:
> > +ÂÂÂÂÂ #qcom,gcc-sm8150
> > +ÂÂÂÂÂ #qcom,gcc-sc7180
> > +ÂÂÂÂÂ - items:
> > +ÂÂÂÂÂÂÂ - const: bi_tcxo
> > +ÂÂÂÂÂÂÂ - const: bi_tcxo_ao
> > +ÂÂÂÂÂÂÂ - const: sleep_clk
>
> Not required for SC7180.

How are you determining this?

Per the earlier discussion with Stephen, if the hardware exists, it 
should be represented in DT.  According to the documentation I see, the 
sleep clock is routed to the GCC on SC7180.  The driver is not required 
to make use of it.  Thus its required from the DT perspective.

> > +ÂÂÂÂÂ #qcom,gcc-msm8998
> > +ÂÂÂÂÂ - items:
> > +ÂÂÂÂÂÂÂ - const: xo
> > +ÂÂÂÂÂÂÂ - const: usb3_pipe
> > +ÂÂÂÂÂÂÂ - const: ufs_rx_symbol0
> > +ÂÂÂÂÂÂÂ - const: ufs_rx_symbol1
> > +ÂÂÂÂÂÂÂ - const: ufs_tx_symbol0
> > +ÂÂÂÂÂÂÂ - const: pcie0_pipe
> > ÂÂÂ '#clock-cells':
> > ÂÂÂÂÂ const: 1
> > @@ -118,6 +136,7 @@ else:
> > ÂÂÂÂÂÂÂ compatible:
> > ÂÂÂÂÂÂÂÂÂ contains:
> > ÂÂÂÂÂÂÂÂÂÂÂ enum:
> > +ÂÂÂÂÂÂÂÂÂÂÂ - qcom,gcc-msm8998
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂ - qcom,gcc-sm8150
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂ - qcom,gcc-sc7180
> > ÂÂÂ then:
> > @@ -179,8 +198,8 @@ examples:
> > ÂÂÂÂÂ clock-controller@100000 {
> > ÂÂÂÂÂÂÂ compatible = "qcom,gcc-sc7180";
> > ÂÂÂÂÂÂÂ reg = <0x100000 0x1f0000>;
> > -ÂÂÂÂÂ clocks = <&rpmhcc 0>, <&rpmhcc 1>;
> > -ÂÂÂÂÂ clock-names = "bi_tcxo", "bi_tcxo_ao";
> > +ÂÂÂÂÂ clocks = <&rpmhcc 0>, <&rpmhcc 1>, <0>;
> > +ÂÂÂÂÂ clock-names = "bi_tcxo", "bi_tcxo_ao", "sleep_clk";
>
> SC7180 does not require a sleep clock.
>
> > ÂÂÂÂÂÂÂ #clock-cells = <1>;
> > ÂÂÂÂÂÂÂ #reset-cells = <1>;
> > ÂÂÂÂÂÂÂ #power-domain-cells = <1>;


--
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.