diff mbox series

[1/7] dt-bindings: clock: Add bindings for versal clock driver

Message ID 1573564580-9006-2-git-send-email-rajan.vaja@xilinx.com
State Changes Requested, archived
Headers show
Series clk: zynqmp: Extend and fix zynqmp clock driver | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema success

Commit Message

Rajan Vaja Nov. 12, 2019, 1:16 p.m. UTC 
Add documentation to describe Xilinx Versal clock driver
bindings.

Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
---
 .../devicetree/bindings/clock/xlnx,versal-clk.yaml |  67 +++++++++++
 include/dt-bindings/clock/xlnx-versal-clk.h        | 123 +++++++++++++++++++++
 2 files changed, 190 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
 create mode 100644 include/dt-bindings/clock/xlnx-versal-clk.h

Comments

Stephen Boyd Nov. 12, 2019, 10:51 p.m. UTC | #1 
Quoting Rajan Vaja (2019-11-12 05:16:14)
> diff --git a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> new file mode 100644
> index 0000000..da82f6a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/clock/xlnx,versal-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx Versal clock controller
> +
> +maintainers:
> +  - Michal Simek <michal.simek@xilinx.com>
> +  - Jolly Shah <jolly.shah@xilinx.com>
> +  - Rajan Vaja <rajan.vaja@xilinx.com>
> +
> +description: |
> +  The clock controller is a h/w block of Xilinx versal clock tree. It reads

hardware instead of h/w

> +  required input clock frequencies from the devicetree and acts as clock
> +  provider for all clock consumers of PS clocks. See clock_bindings.txt
> +  for more information on the generic clock bindings.

Please drop this last sentence about clock_bindings.txt

> +
> +properties:
> +  compatible:
> +    const: xlnx,versal-clk
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  clocks:
> +    description: List of clock specifiers which are external input
> +      clocks to the given clock controller.
> +    minItems: 3
> +    maxItems: 3
> +    items:
> +      - description: ref clk
> +      - description: alternate ref clk
> +      - description: pl alternate ref clk

What is "pl"? Can you clarify?

> +
> +  clock-names:
> +    minItems: 3
> +    maxItems: 3
> +    items:
> +      - const: ref_clk
> +      - const: alt_ref_clk
> +      - const: pl_alt_ref_clk
> +
> +required:
> +  - compatible
> +  - "#clock-cells"
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    firmware {
> +      zynqmp_firmware: zynqmp-firmware {
> +        compatible = "xlnx,zynqmp-firmware";
> +        method = "smc";

Is there a way to say in the binding that this must be a child of a
xlnx,zynqmp-firmware node? That would be ideal so we can constrain this
to that location somehow.

> +        versal_clk: clock-controller {
> +          #clock-cells = <1>;
> +          compatible = "xlnx,versal-clk";
> +          clocks = <&ref_clk>, <&alt_ref_clk>, <&pl_alt_ref_clk>;
> +          clock-names = "ref_clk", "alt_ref_clk", "pl_alt_ref_clk";
> +        };
> +      };
> +    };
> +...
Rob Herring Nov. 18, 2019, 5:30 p.m. UTC | #2 
On Tue, Nov 12, 2019 at 02:51:46PM -0800, Stephen Boyd wrote:
> Quoting Rajan Vaja (2019-11-12 05:16:14)
> > diff --git a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> > new file mode 100644
> > index 0000000..da82f6a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/bindings/clock/xlnx,versal-clk.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx Versal clock controller
> > +
> > +maintainers:
> > +  - Michal Simek <michal.simek@xilinx.com>
> > +  - Jolly Shah <jolly.shah@xilinx.com>
> > +  - Rajan Vaja <rajan.vaja@xilinx.com>
> > +
> > +description: |
> > +  The clock controller is a h/w block of Xilinx versal clock tree. It reads
> 
> hardware instead of h/w
> 
> > +  required input clock frequencies from the devicetree and acts as clock
> > +  provider for all clock consumers of PS clocks. See clock_bindings.txt
> > +  for more information on the generic clock bindings.
> 
> Please drop this last sentence about clock_bindings.txt
> 
> > +
> > +properties:
> > +  compatible:
> > +    const: xlnx,versal-clk
> > +
> > +  "#clock-cells":
> > +    const: 1
> > +
> > +  clocks:
> > +    description: List of clock specifiers which are external input
> > +      clocks to the given clock controller.
> > +    minItems: 3
> > +    maxItems: 3

Can drop these. Implied by by 'items' list.

> > +    items:
> > +      - description: ref clk
> > +      - description: alternate ref clk
> > +      - description: pl alternate ref clk
> 
> What is "pl"? Can you clarify?
> 
> > +
> > +  clock-names:
> > +    minItems: 3
> > +    maxItems: 3

Same here.

> > +    items:
> > +      - const: ref_clk
> > +      - const: alt_ref_clk
> > +      - const: pl_alt_ref_clk

'_clk' is redundant.

> > +
> > +required:
> > +  - compatible
> > +  - "#clock-cells"
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    firmware {
> > +      zynqmp_firmware: zynqmp-firmware {
> > +        compatible = "xlnx,zynqmp-firmware";
> > +        method = "smc";
> 
> Is there a way to say in the binding that this must be a child of a
> xlnx,zynqmp-firmware node? That would be ideal so we can constrain this
> to that location somehow.

Yes. Add the node name as a property to the f/w schema and reference 
($ref) this file and add 'select: false' to this one. The problem is the 
firmware binding is probably not yet a schema. Once it is a schema, this 
example will start failing because it's incomplete. For that reason, I 
prefer the examples in these cases (inc MFDs) in the base schema and not 
in the child node schemas.

> > +        versal_clk: clock-controller {
> > +          #clock-cells = <1>;
> > +          compatible = "xlnx,versal-clk";
> > +          clocks = <&ref_clk>, <&alt_ref_clk>, <&pl_alt_ref_clk>;
> > +          clock-names = "ref_clk", "alt_ref_clk", "pl_alt_ref_clk";
> > +        };
> > +      };
> > +    };
> > +...
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
new file mode 100644
index 0000000..da82f6a
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
@@ -0,0 +1,67 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/clock/xlnx,versal-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Versal clock controller
+
+maintainers:
+  - Michal Simek <michal.simek@xilinx.com>
+  - Jolly Shah <jolly.shah@xilinx.com>
+  - Rajan Vaja <rajan.vaja@xilinx.com>
+
+description: |
+  The clock controller is a h/w block of Xilinx versal clock tree. It reads
+  required input clock frequencies from the devicetree and acts as clock
+  provider for all clock consumers of PS clocks. See clock_bindings.txt
+  for more information on the generic clock bindings.
+
+properties:
+  compatible:
+    const: xlnx,versal-clk
+
+  "#clock-cells":
+    const: 1
+
+  clocks:
+    description: List of clock specifiers which are external input
+      clocks to the given clock controller.
+    minItems: 3
+    maxItems: 3
+    items:
+      - description: ref clk
+      - description: alternate ref clk
+      - description: pl alternate ref clk
+
+  clock-names:
+    minItems: 3
+    maxItems: 3
+    items:
+      - const: ref_clk
+      - const: alt_ref_clk
+      - const: pl_alt_ref_clk
+
+required:
+  - compatible
+  - "#clock-cells"
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    firmware {
+      zynqmp_firmware: zynqmp-firmware {
+        compatible = "xlnx,zynqmp-firmware";
+        method = "smc";
+        versal_clk: clock-controller {
+          #clock-cells = <1>;
+          compatible = "xlnx,versal-clk";
+          clocks = <&ref_clk>, <&alt_ref_clk>, <&pl_alt_ref_clk>;
+          clock-names = "ref_clk", "alt_ref_clk", "pl_alt_ref_clk";
+        };
+      };
+    };
+...
diff --git a/include/dt-bindings/clock/xlnx-versal-clk.h b/include/dt-bindings/clock/xlnx-versal-clk.h
new file mode 100644
index 0000000..264d634
--- /dev/null
+++ b/include/dt-bindings/clock/xlnx-versal-clk.h
@@ -0,0 +1,123 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Copyright (C) 2019 Xilinx Inc.
+ *
+ */
+
+#ifndef _DT_BINDINGS_CLK_VERSAL_H
+#define _DT_BINDINGS_CLK_VERSAL_H
+
+#define PMC_PLL					1
+#define APU_PLL					2
+#define RPU_PLL					3
+#define CPM_PLL					4
+#define NOC_PLL					5
+#define PLL_MAX					6
+#define PMC_PRESRC				7
+#define PMC_POSTCLK				8
+#define PMC_PLL_OUT				9
+#define PPLL					10
+#define NOC_PRESRC				11
+#define NOC_POSTCLK				12
+#define NOC_PLL_OUT				13
+#define NPLL					14
+#define APU_PRESRC				15
+#define APU_POSTCLK				16
+#define APU_PLL_OUT				17
+#define APLL					18
+#define RPU_PRESRC				19
+#define RPU_POSTCLK				20
+#define RPU_PLL_OUT				21
+#define RPLL					22
+#define CPM_PRESRC				23
+#define CPM_POSTCLK				24
+#define CPM_PLL_OUT				25
+#define CPLL					26
+#define PPLL_TO_XPD				27
+#define NPLL_TO_XPD				28
+#define APLL_TO_XPD				29
+#define RPLL_TO_XPD				30
+#define EFUSE_REF				31
+#define SYSMON_REF				32
+#define IRO_SUSPEND_REF				33
+#define USB_SUSPEND				34
+#define SWITCH_TIMEOUT				35
+#define RCLK_PMC				36
+#define RCLK_LPD				37
+#define WDT					38
+#define TTC0					39
+#define TTC1					40
+#define TTC2					41
+#define TTC3					42
+#define GEM_TSU					43
+#define GEM_TSU_LB				44
+#define MUXED_IRO_DIV2				45
+#define MUXED_IRO_DIV4				46
+#define PSM_REF					47
+#define GEM0_RX					48
+#define GEM0_TX					49
+#define GEM1_RX					50
+#define GEM1_TX					51
+#define CPM_CORE_REF				52
+#define CPM_LSBUS_REF				53
+#define CPM_DBG_REF				54
+#define CPM_AUX0_REF				55
+#define CPM_AUX1_REF				56
+#define QSPI_REF				57
+#define OSPI_REF				58
+#define SDIO0_REF				59
+#define SDIO1_REF				60
+#define PMC_LSBUS_REF				61
+#define I2C_REF					62
+#define TEST_PATTERN_REF			63
+#define DFT_OSC_REF				64
+#define PMC_PL0_REF				65
+#define PMC_PL1_REF				66
+#define PMC_PL2_REF				67
+#define PMC_PL3_REF				68
+#define CFU_REF					69
+#define SPARE_REF				70
+#define NPI_REF					71
+#define HSM0_REF				72
+#define HSM1_REF				73
+#define SD_DLL_REF				74
+#define FPD_TOP_SWITCH				75
+#define FPD_LSBUS				76
+#define ACPU					77
+#define DBG_TRACE				78
+#define DBG_FPD					79
+#define LPD_TOP_SWITCH				80
+#define ADMA					81
+#define LPD_LSBUS				82
+#define CPU_R5					83
+#define CPU_R5_CORE				84
+#define CPU_R5_OCM				85
+#define CPU_R5_OCM2				86
+#define IOU_SWITCH				87
+#define GEM0_REF				88
+#define GEM1_REF				89
+#define GEM_TSU_REF				90
+#define USB0_BUS_REF				91
+#define UART0_REF				92
+#define UART1_REF				93
+#define SPI0_REF				94
+#define SPI1_REF				95
+#define CAN0_REF				96
+#define CAN1_REF				97
+#define I2C0_REF				98
+#define I2C1_REF				99
+#define DBG_LPD					100
+#define TIMESTAMP_REF				101
+#define DBG_TSTMP				102
+#define CPM_TOPSW_REF				103
+#define USB3_DUAL_REF				104
+#define OUTCLK_MAX				105
+#define REF_CLK					106
+#define PL_ALT_REF_CLK				107
+#define MUXED_IRO				108
+#define PL_EXT					109
+#define PL_LB					110
+#define MIO_50_OR_51				111
+#define MIO_24_OR_25				112
+
+#endif