diff mbox series

[v3,07/15] dt-bindings: clock: Fix qcom,gpucc bindings for sdm845/sc7180/msm8998

Message ID 20200130131220.v3.7.I513cd73b16665065ae6c22cf594d8b543745e28c@changeid (mailing list archive)
State Superseded
Headers show
Series clk: qcom: Fix parenting for dispcc/gpucc/videocc | expand

Commit Message

Doug Anderson Jan. 30, 2020, 9:12 p.m. UTC
The qcom,gpucc bindings had a few problems with them:

1. When things were converted to yaml the name of the "gpll0 main"
   clock got changed from "gpll0" to "gpll0_main".  Change it back for
   msm8998.

2. Apparently there is a push not to use purist aliases for clocks but
   instead to just use the internal Qualcomm names.  For sdm845 and
   sc7180 (where the drivers haven't already been changed) move in
   this direction.

Things were also getting complicated harder to deal with by jamming
several SoCs into one file.  Splitting simplifies things.

Fixes: 5c6f3a36b913 ("dt-bindings: clock: Add YAML schemas for the QCOM GPUCC clock bindings")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v3:
- Added pointer to inlude file in description.
- Everyone but msm8998 now uses internal QC names.
- Fixed typo grpahics => graphics
- Split bindings into 3 files.

Changes in v2:
- Patch ("dt-bindings: clock: Fix qcom,gpucc...") new for v2.

 .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 -------------------
 .../bindings/clock/qcom,msm8998-gpucc.yaml    | 66 +++++++++++++++++
 .../bindings/clock/qcom,sc7180-gpucc.yaml     | 72 +++++++++++++++++++
 .../bindings/clock/qcom,sdm845-gpucc.yaml     | 72 +++++++++++++++++++
 4 files changed, 210 insertions(+), 72 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml

Comments

Rob Herring (Arm) Jan. 31, 2020, 4:43 p.m. UTC | #1
On Thu, Jan 30, 2020 at 3:12 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> The qcom,gpucc bindings had a few problems with them:
>
> 1. When things were converted to yaml the name of the "gpll0 main"
>    clock got changed from "gpll0" to "gpll0_main".  Change it back for
>    msm8998.
>
> 2. Apparently there is a push not to use purist aliases for clocks but
>    instead to just use the internal Qualcomm names.  For sdm845 and
>    sc7180 (where the drivers haven't already been changed) move in
>    this direction.
>
> Things were also getting complicated harder to deal with by jamming
> several SoCs into one file.  Splitting simplifies things.
>
> Fixes: 5c6f3a36b913 ("dt-bindings: clock: Add YAML schemas for the QCOM GPUCC clock bindings")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v3:
> - Added pointer to inlude file in description.
> - Everyone but msm8998 now uses internal QC names.
> - Fixed typo grpahics => graphics
> - Split bindings into 3 files.
>
> Changes in v2:
> - Patch ("dt-bindings: clock: Fix qcom,gpucc...") new for v2.
>
>  .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 -------------------
>  .../bindings/clock/qcom,msm8998-gpucc.yaml    | 66 +++++++++++++++++
>  .../bindings/clock/qcom,sc7180-gpucc.yaml     | 72 +++++++++++++++++++
>  .../bindings/clock/qcom,sdm845-gpucc.yaml     | 72 +++++++++++++++++++
>  4 files changed, 210 insertions(+), 72 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml

I'm not seeing any differences in sdm845 and sc7180. Do those really
need to be separate? It doesn't have to be all combined or all
separate.

Rob
Doug Anderson Jan. 31, 2020, 4:48 p.m. UTC | #2
Hi,

On Fri, Jan 31, 2020 at 8:43 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Jan 30, 2020 at 3:12 PM Douglas Anderson <dianders@chromium.org> wrote:
> >
> > The qcom,gpucc bindings had a few problems with them:
> >
> > 1. When things were converted to yaml the name of the "gpll0 main"
> >    clock got changed from "gpll0" to "gpll0_main".  Change it back for
> >    msm8998.
> >
> > 2. Apparently there is a push not to use purist aliases for clocks but
> >    instead to just use the internal Qualcomm names.  For sdm845 and
> >    sc7180 (where the drivers haven't already been changed) move in
> >    this direction.
> >
> > Things were also getting complicated harder to deal with by jamming
> > several SoCs into one file.  Splitting simplifies things.
> >
> > Fixes: 5c6f3a36b913 ("dt-bindings: clock: Add YAML schemas for the QCOM GPUCC clock bindings")
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> > Changes in v3:
> > - Added pointer to inlude file in description.
> > - Everyone but msm8998 now uses internal QC names.
> > - Fixed typo grpahics => graphics
> > - Split bindings into 3 files.
> >
> > Changes in v2:
> > - Patch ("dt-bindings: clock: Fix qcom,gpucc...") new for v2.
> >
> >  .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 -------------------
> >  .../bindings/clock/qcom,msm8998-gpucc.yaml    | 66 +++++++++++++++++
> >  .../bindings/clock/qcom,sc7180-gpucc.yaml     | 72 +++++++++++++++++++
> >  .../bindings/clock/qcom,sdm845-gpucc.yaml     | 72 +++++++++++++++++++
> >  4 files changed, 210 insertions(+), 72 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml
> >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml
> >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml
>
> I'm not seeing any differences in sdm845 and sc7180. Do those really
> need to be separate? It doesn't have to be all combined or all
> separate.

They are the same, other than pointing to a different #include file.
I debated whether to put them in one file (arbitrarily named after one
SoC or the other) or to put them in individual files.  I got the
impression from Stephen that he'd prefer them to be separate files
even in the case that they were 99% identical, but I certainly could
have misunderstood.

I'll do whatever you guys agree to.  If you want them in one file I'll
probably name it "qcom,sdm845-gpucc.yaml" just because that SoC is
earlier, unless someone tells me otherwise.

-Doug
Doug Anderson Feb. 3, 2020, 6:35 p.m. UTC | #3
Hi,


On Mon, Feb 3, 2020 at 8:29 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Doug Anderson (2020-01-31 08:48:37)
> > Hi,
> >
> > On Fri, Jan 31, 2020 at 8:43 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Thu, Jan 30, 2020 at 3:12 PM Douglas Anderson <dianders@chromium.org> wrote:
> > > >
> > > > The qcom,gpucc bindings had a few problems with them:
> > > >
> > > > 1. When things were converted to yaml the name of the "gpll0 main"
> > > >    clock got changed from "gpll0" to "gpll0_main".  Change it back for
> > > >    msm8998.
> > > >
> > > > 2. Apparently there is a push not to use purist aliases for clocks but
> > > >    instead to just use the internal Qualcomm names.  For sdm845 and
> > > >    sc7180 (where the drivers haven't already been changed) move in
> > > >    this direction.
> > > >
> > > > Things were also getting complicated harder to deal with by jamming
> > > > several SoCs into one file.  Splitting simplifies things.
> > > >
> > > > Fixes: 5c6f3a36b913 ("dt-bindings: clock: Add YAML schemas for the QCOM GPUCC clock bindings")
> > > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > > ---
> > > >
> > > > Changes in v3:
> > > > - Added pointer to inlude file in description.
> > > > - Everyone but msm8998 now uses internal QC names.
> > > > - Fixed typo grpahics => graphics
> > > > - Split bindings into 3 files.
> > > >
> > > > Changes in v2:
> > > > - Patch ("dt-bindings: clock: Fix qcom,gpucc...") new for v2.
> > > >
> > > >  .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 -------------------
> > > >  .../bindings/clock/qcom,msm8998-gpucc.yaml    | 66 +++++++++++++++++
> > > >  .../bindings/clock/qcom,sc7180-gpucc.yaml     | 72 +++++++++++++++++++
> > > >  .../bindings/clock/qcom,sdm845-gpucc.yaml     | 72 +++++++++++++++++++
> > > >  4 files changed, 210 insertions(+), 72 deletions(-)
> > > >  delete mode 100644 Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> > > >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml
> > > >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml
> > > >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml
> > >
> > > I'm not seeing any differences in sdm845 and sc7180. Do those really
> > > need to be separate? It doesn't have to be all combined or all
> > > separate.
> >
> > They are the same, other than pointing to a different #include file.
> > I debated whether to put them in one file (arbitrarily named after one
> > SoC or the other) or to put them in individual files.  I got the
> > impression from Stephen that he'd prefer them to be separate files
> > even in the case that they were 99% identical, but I certainly could
> > have misunderstood.
> >
> > I'll do whatever you guys agree to.  If you want them in one file I'll
> > probably name it "qcom,sdm845-gpucc.yaml" just because that SoC is
> > earlier, unless someone tells me otherwise.
> >
>
> I'd prefer them to be split out and point at the include file so we know
> what numbers are valid. It provides clarity and helps avoid the back and
> forth of combining and splitting the files. We suffer the same problem
> on the driver side, and we've long given up trying to combine SoCs when
> they're otherwise fairly similar.

Thanks for clarifying!  Rob: I hope it's OK that I've gone ahead and
sent out v4 leaving this alone.  I knew you were interested in getting
the other bindings patch out sooner rather than later and I was hoping
to get both series out together so I could context switch to a few
other things early this week.  Apologies if this was moving too
fast...

-Doug
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
deleted file mode 100644
index 622845aa643f..000000000000
--- a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
+++ /dev/null
@@ -1,72 +0,0 @@ 
-# SPDX-License-Identifier: GPL-2.0-only
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/bindings/clock/qcom,gpucc.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Qualcomm Graphics Clock & Reset Controller Binding
-
-maintainers:
-  - Taniya Das <tdas@codeaurora.org>
-
-description: |
-  Qualcomm grpahics clock control module which supports the clocks, resets and
-  power domains.
-
-properties:
-  compatible:
-    enum:
-      - qcom,msm8998-gpucc
-      - qcom,sc7180-gpucc
-      - qcom,sdm845-gpucc
-
-  clocks:
-    minItems: 1
-    maxItems: 3
-    items:
-      - description: Board XO source
-      - description: GPLL0 main branch source from GCC(gcc_gpu_gpll0_clk_src)
-      - description: GPLL0 div branch source from GCC(gcc_gpu_gpll0_div_clk_src)
-
-  clock-names:
-    minItems: 1
-    maxItems: 3
-    items:
-      - const: xo
-      - const: gpll0_main
-      - const: gpll0_div
-
-  '#clock-cells':
-    const: 1
-
-  '#reset-cells':
-    const: 1
-
-  '#power-domain-cells':
-    const: 1
-
-  reg:
-    maxItems: 1
-
-required:
-  - compatible
-  - reg
-  - clocks
-  - clock-names
-  - '#clock-cells'
-  - '#reset-cells'
-  - '#power-domain-cells'
-
-examples:
-  # Example of GPUCC with clock node properties for SDM845:
-  - |
-    clock-controller@5090000 {
-      compatible = "qcom,sdm845-gpucc";
-      reg = <0x5090000 0x9000>;
-      clocks = <&rpmhcc 0>, <&gcc 31>, <&gcc 32>;
-      clock-names = "xo", "gpll0_main", "gpll0_div";
-      #clock-cells = <1>;
-      #reset-cells = <1>;
-      #power-domain-cells = <1>;
-     };
-...
diff --git a/Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml
new file mode 100644
index 000000000000..482e36b74cea
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml
@@ -0,0 +1,66 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/clock/qcom,msm8998-gpucc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Graphics Clock & Reset Controller Binding for MSM8998
+
+maintainers:
+  - Taniya Das <tdas@codeaurora.org>
+
+description: |
+  Qualcomm graphics clock control module which supports the clocks, resets and
+  power domains on MSM8998.
+
+  See also dt-bindings/clock/qcom,gpucc-msm8998.h.
+
+properties:
+  compatible:
+    const: qcom,msm8998-gpucc
+
+  clocks:
+    items:
+      - description: Board XO source
+      - description: GPLL0 main branch source (gcc_gpu_gpll0_clk_src)
+
+  clock-names:
+    items:
+      - const: xo
+      - const: gpll0
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - '#reset-cells'
+  - '#power-domain-cells'
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-msm8998.h>
+    #include <dt-bindings/clock/qcom,rpmcc.h>
+    clock-controller@5065000 {
+      compatible = "qcom,msm8998-gpucc";
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+      #power-domain-cells = <1>;
+      reg = <0x05065000 0x9000>;
+      clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>, <&gcc GPLL0_OUT_MAIN>;
+      clock-names = "xo", "gpll0";
+    };
+...
diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml
new file mode 100644
index 000000000000..de42d68162d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml
@@ -0,0 +1,72 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/clock/qcom,sc7180-gpucc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Graphics Clock & Reset Controller Binding for SC7180
+
+maintainers:
+  - Taniya Das <tdas@codeaurora.org>
+
+description: |
+  Qualcomm graphics clock control module which supports the clocks, resets and
+  power domains on SC7180.
+
+  See also dt-bindings/clock/qcom,gpucc-sc7180.h.
+
+properties:
+  compatible:
+    const: qcom,sc7180-gpucc
+
+  clocks:
+    items:
+      - description: Board XO source
+      - description: GPLL0 main branch source
+      - description: GPLL0 div branch source
+
+  clock-names:
+    items:
+      - const: bi_tcxo
+      - const: gcc_gpu_gpll0_clk_src
+      - const: gcc_gpu_gpll0_div_clk_src
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - '#reset-cells'
+  - '#power-domain-cells'
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
+    #include <dt-bindings/clock/qcom,rpmh.h>
+    clock-controller@5090000 {
+      compatible = "qcom,sc7180-gpucc";
+      reg = <0 0x05090000 0 0x9000>;
+      clocks = <&rpmhcc RPMH_CXO_CLK>,
+               <&gcc GCC_GPU_GPLL0_CLK_SRC>,
+               <&gcc GCC_GPU_GPLL0_DIV_CLK_SRC>;
+      clock-names = "bi_tcxo",
+                    "gcc_gpu_gpll0_clk_src",
+                    "gcc_gpu_gpll0_div_clk_src";
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+      #power-domain-cells = <1>;
+    };
+...
diff --git a/Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml
new file mode 100644
index 000000000000..45fb7eb2dc34
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml
@@ -0,0 +1,72 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/clock/qcom,sdm845-gpucc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Graphics Clock & Reset Controller Binding for SDM845
+
+maintainers:
+  - Taniya Das <tdas@codeaurora.org>
+
+description: |
+  Qualcomm graphics clock control module which supports the clocks, resets and
+  power domains on SDM845.
+
+  See also dt-bindings/clock/qcom,gpucc-sdm845.h.
+
+properties:
+  compatible:
+    const: qcom,sdm845-gpucc
+
+  clocks:
+    items:
+      - description: Board XO source
+      - description: GPLL0 main branch source
+      - description: GPLL0 div branch source
+
+  clock-names:
+    items:
+      - const: bi_tcxo
+      - const: gcc_gpu_gpll0_clk_src
+      - const: gcc_gpu_gpll0_div_clk_src
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - '#reset-cells'
+  - '#power-domain-cells'
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+    #include <dt-bindings/clock/qcom,rpmh.h>
+    clock-controller@5090000 {
+      compatible = "qcom,sdm845-gpucc";
+      reg = <0 0x05090000 0 0x9000>;
+      clocks = <&rpmhcc RPMH_CXO_CLK>,
+               <&gcc GCC_GPU_GPLL0_CLK_SRC>,
+               <&gcc GCC_GPU_GPLL0_DIV_CLK_SRC>;
+      clock-names = "bi_tcxo",
+                    "gcc_gpu_gpll0_clk_src",
+                    "gcc_gpu_gpll0_div_clk_src";
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+      #power-domain-cells = <1>;
+    };
+...