diff mbox series

[v2,03/15] dt-bindings: clock: Document qcom,gcc-ipq8064 binding

Message ID 20220120232028.6738-4-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Headers show
Series Multiple addition and improvement to ipq8064 gcc | expand

Commit Message

Christian Marangi Jan. 20, 2022, 11:20 p.m. UTC 
Document qcom,gcc-ipq8064 binding needed to declare pxo and cxo source
clocks. The gcc node is also used by the tsens driver, already documented,
to get the calib nvmem cells and the base reg from gcc.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../bindings/clock/qcom,gcc-ipq8064.yaml      | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml

Comments

Rob Herring Jan. 21, 2022, 1:37 a.m. UTC | #1 
On Fri, 21 Jan 2022 00:20:16 +0100, Ansuel Smith wrote:
> Document qcom,gcc-ipq8064 binding needed to declare pxo and cxo source
> clocks. The gcc node is also used by the tsens driver, already documented,
> to get the calib nvmem cells and the base reg from gcc.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../bindings/clock/qcom,gcc-ipq8064.yaml      | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.example.dt.yaml: clock-controller@900000: compatible: ['qcom,gcc-ipq8064', 'syscon'] is too long
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/qcom,gcc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.example.dt.yaml: clock-controller@900000: compatible: Additional items are not allowed ('syscon' was unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/qcom,gcc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.example.dt.yaml: clock-controller@900000: 'clock-names', 'clocks', 'thermal-sensor' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/qcom,gcc.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1582346

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Rob Herring Jan. 21, 2022, 2:04 a.m. UTC | #2 
On Fri, Jan 21, 2022 at 12:20:16AM +0100, Ansuel Smith wrote:
> Document qcom,gcc-ipq8064 binding needed to declare pxo and cxo source
> clocks. The gcc node is also used by the tsens driver, already documented,
> to get the calib nvmem cells and the base reg from gcc.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../bindings/clock/qcom,gcc-ipq8064.yaml      | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> new file mode 100644
> index 000000000000..abc76a46b2ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,gcc-ipq8064.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Global Clock & Reset Controller Binding for IPQ8064
> +
> +allOf:
> +  - $ref: qcom,gcc.yaml#
> +
> +maintainers:
> +  - Ansuel Smith <ansuelsmth@gmail.com>
> +
> +description: |
> +  Qualcomm global clock control module which supports the clocks, resets and
> +  power domains on IPQ8064.
> +
> +  See also:
> +  - dt-bindings/clock/qcom,gcc-ipq806x.h
> +  - dt-bindings/reset/qcom,gcc-ipq806x.h
> +
> +properties:

This schema will never be applied because there is not a compatible 
property to use for matching. The base/common schema is the one that 
shouldn't have a compatible and then the specific schemas like this 
one do.

> +  clocks:
> +    items:
> +      - description: PXO source
> +      - description: CXO source
> +
> +  clock-names:
> +    items:
> +      - const: pxo
> +      - const: cxo
> +
> +  thermal-sensor:
> +    type: object
> +
> +    allOf:
> +      - $ref: /schemas/thermal/qcom-tsens.yaml#
> +
> +required:
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    gcc: clock-controller@900000 {
> +      compatible = "qcom,gcc-ipq8064", "syscon";
> +      reg = <0x00900000 0x4000>;
> +      clocks = <&pxo_board>, <&cxo_board>;
> +      clock-names = "pxo", "cxo";
> +      #clock-cells = <1>;
> +      #reset-cells = <1>;
> +      #power-domain-cells = <1>;
> +
> +      tsens: thermal-sensor {
> +        compatible = "qcom,ipq8064-tsens";
> +
> +        nvmem-cells = <&tsens_calib>, <&tsens_calib_backup>;
> +        nvmem-cell-names = "calib", "calib_backup";
> +        interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-names = "uplow";
> +
> +        #qcom,sensors = <11>;
> +        #thermal-sensor-cells = <1>;
> +      };
> +    };
> -- 
> 2.33.1
> 
>
Christian Marangi Jan. 21, 2022, 7:07 p.m. UTC | #3 
On Thu, Jan 20, 2022 at 08:04:03PM -0600, Rob Herring wrote:
> On Fri, Jan 21, 2022 at 12:20:16AM +0100, Ansuel Smith wrote:
> > Document qcom,gcc-ipq8064 binding needed to declare pxo and cxo source
> > clocks. The gcc node is also used by the tsens driver, already documented,
> > to get the calib nvmem cells and the base reg from gcc.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  .../bindings/clock/qcom,gcc-ipq8064.yaml      | 70 +++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> > new file mode 100644
> > index 000000000000..abc76a46b2ca
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> > @@ -0,0 +1,70 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/qcom,gcc-ipq8064.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Global Clock & Reset Controller Binding for IPQ8064
> > +
> > +allOf:
> > +  - $ref: qcom,gcc.yaml#
> > +
> > +maintainers:
> > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > +
> > +description: |
> > +  Qualcomm global clock control module which supports the clocks, resets and
> > +  power domains on IPQ8064.
> > +
> > +  See also:
> > +  - dt-bindings/clock/qcom,gcc-ipq806x.h
> > +  - dt-bindings/reset/qcom,gcc-ipq806x.h
> > +
> > +properties:
> 
> This schema will never be applied because there is not a compatible 
> property to use for matching. The base/common schema is the one that 
> shouldn't have a compatible and then the specific schemas like this 
> one do.
>

Just to make things clear. To fix things up, what changes should I do?
- I should remove the compatible from the base schema qcom,gcc.yaml
- Add the compatible to this schema
- Create another schema that includes all the others compatible?

Can I instead:
- Create a qcom,gcc-common.yaml schema
- Modify the qcom,gcc.yaml schema to ref the common one and drop the
  other binding.
- Fix this schema with the missing compatible?

Tell me how I should proceed since it looks to me that all the
Documentation for the gcc driver looks a bit mess and full of
duplicated stuff.

> > +  clocks:
> > +    items:
> > +      - description: PXO source
> > +      - description: CXO source
> > +
> > +  clock-names:
> > +    items:
> > +      - const: pxo
> > +      - const: cxo
> > +
> > +  thermal-sensor:
> > +    type: object
> > +
> > +    allOf:
> > +      - $ref: /schemas/thermal/qcom-tsens.yaml#
> > +
> > +required:
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    gcc: clock-controller@900000 {
> > +      compatible = "qcom,gcc-ipq8064", "syscon";
> > +      reg = <0x00900000 0x4000>;
> > +      clocks = <&pxo_board>, <&cxo_board>;
> > +      clock-names = "pxo", "cxo";
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> > +      #power-domain-cells = <1>;
> > +
> > +      tsens: thermal-sensor {
> > +        compatible = "qcom,ipq8064-tsens";
> > +
> > +        nvmem-cells = <&tsens_calib>, <&tsens_calib_backup>;
> > +        nvmem-cell-names = "calib", "calib_backup";
> > +        interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
> > +        interrupt-names = "uplow";
> > +
> > +        #qcom,sensors = <11>;
> > +        #thermal-sensor-cells = <1>;
> > +      };
> > +    };
> > -- 
> > 2.33.1
> > 
> >
Rob Herring Jan. 21, 2022, 7:22 p.m. UTC | #4 
On Fri, Jan 21, 2022 at 1:07 PM Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> On Thu, Jan 20, 2022 at 08:04:03PM -0600, Rob Herring wrote:
> > On Fri, Jan 21, 2022 at 12:20:16AM +0100, Ansuel Smith wrote:
> > > Document qcom,gcc-ipq8064 binding needed to declare pxo and cxo source
> > > clocks. The gcc node is also used by the tsens driver, already documented,
> > > to get the calib nvmem cells and the base reg from gcc.
> > >
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > ---
> > >  .../bindings/clock/qcom,gcc-ipq8064.yaml      | 70 +++++++++++++++++++
> > >  1 file changed, 70 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> > > new file mode 100644
> > > index 000000000000..abc76a46b2ca
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
> > > @@ -0,0 +1,70 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/clock/qcom,gcc-ipq8064.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Qualcomm Global Clock & Reset Controller Binding for IPQ8064
> > > +
> > > +allOf:
> > > +  - $ref: qcom,gcc.yaml#
> > > +
> > > +maintainers:
> > > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > > +
> > > +description: |
> > > +  Qualcomm global clock control module which supports the clocks, resets and
> > > +  power domains on IPQ8064.
> > > +
> > > +  See also:
> > > +  - dt-bindings/clock/qcom,gcc-ipq806x.h
> > > +  - dt-bindings/reset/qcom,gcc-ipq806x.h
> > > +
> > > +properties:
> >
> > This schema will never be applied because there is not a compatible
> > property to use for matching. The base/common schema is the one that
> > shouldn't have a compatible and then the specific schemas like this
> > one do.
> >
>
> Just to make things clear. To fix things up, what changes should I do?
> - I should remove the compatible from the base schema qcom,gcc.yaml
> - Add the compatible to this schema
> - Create another schema that includes all the others compatible?

Yes.

>
> Can I instead:
> - Create a qcom,gcc-common.yaml schema
> - Modify the qcom,gcc.yaml schema to ref the common one and drop the
>   other binding.
> - Fix this schema with the missing compatible?

That's fine. That's just a difference in filenames, right?

> Tell me how I should proceed since it looks to me that all the
> Documentation for the gcc driver looks a bit mess and full of
> duplicated stuff.

I think it was originally one document, but had too many if/then
schemas. Or maybe that was another QCom schema. There's no hard rule
on whether to split or not. It's a judgment call.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
new file mode 100644
index 000000000000..abc76a46b2ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml
@@ -0,0 +1,70 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,gcc-ipq8064.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Global Clock & Reset Controller Binding for IPQ8064
+
+allOf:
+  - $ref: qcom,gcc.yaml#
+
+maintainers:
+  - Ansuel Smith <ansuelsmth@gmail.com>
+
+description: |
+  Qualcomm global clock control module which supports the clocks, resets and
+  power domains on IPQ8064.
+
+  See also:
+  - dt-bindings/clock/qcom,gcc-ipq806x.h
+  - dt-bindings/reset/qcom,gcc-ipq806x.h
+
+properties:
+  clocks:
+    items:
+      - description: PXO source
+      - description: CXO source
+
+  clock-names:
+    items:
+      - const: pxo
+      - const: cxo
+
+  thermal-sensor:
+    type: object
+
+    allOf:
+      - $ref: /schemas/thermal/qcom-tsens.yaml#
+
+required:
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    gcc: clock-controller@900000 {
+      compatible = "qcom,gcc-ipq8064", "syscon";
+      reg = <0x00900000 0x4000>;
+      clocks = <&pxo_board>, <&cxo_board>;
+      clock-names = "pxo", "cxo";
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+      #power-domain-cells = <1>;
+
+      tsens: thermal-sensor {
+        compatible = "qcom,ipq8064-tsens";
+
+        nvmem-cells = <&tsens_calib>, <&tsens_calib_backup>;
+        nvmem-cell-names = "calib", "calib_backup";
+        interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "uplow";
+
+        #qcom,sensors = <11>;
+        #thermal-sensor-cells = <1>;
+      };
+    };