[PATCH 1/5] dt-bindings: clk: Add Baikal-T1 CCU PLLs bindings

[PATCH 1/5] dt-bindings: clk: Add Baikal-T1 CCU PLLs bindings

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Baikal-T1 Clocks Control Unit is responsible for transformation of a
signal coming from an external oscillator into clocks of various
frequencies to propagate them then to the corresponding clocks
consumers (either individual IP-blocks or clock domains). In order
to create a set of high-frequency clocks the external signal is
firstly handled by the embedded into CCU PLLs. So the corresponding
dts-node is just a normal clock-provider node with standard set of
properties.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 .../bindings/clock/be,bt1-ccu-pll.yaml        | 139 ++++++++++++++++++
 include/dt-bindings/clock/bt1-ccu.h           |  17 +++
 2 files changed, 156 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
 create mode 100644 include/dt-bindings/clock/bt1-ccu.h

diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
new file mode 100644
index 000000000000..f2e397cc147b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
+#
+# Baikal-T1 Clocks Control Unit PLL Device Tree Bindings.
+#
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/be,bt1-ccu-pll.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Baikal-T1 Clock Control Unit PLLs
+
+maintainers:
+  - Serge Semin <fancer.lancer@gmail.com>
+
+description: |
+  Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
+  subsystems clocking and resetting. The CCU is connected with an external
+  fixed rate oscillator, which signal is transformed into clocks of various
+  frequencies and then propagated to either individual IP-blocks or to groups
+  of blocks (clock domains). The transformation is done by means of PLLs and
+  gateable/non-gateable dividers embedded into the CCU. It's logically divided
+  into the next components:
+  1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
+     in general can provide any frequency supported by the CCU PLLs).
+  2) PLLs clocks generators (PLLs) - described in this bindings file.
+  3) AXI-bus clock dividers (AXI).
+  4) System devices reference clock dividers (SYS).
+  which are connected with each other as shown on the next figure:
+          +---------------+
+          | Baikal-T1 CCU |
+          |   +----+------|- MIPS P5600 cores
+          | +-|PLLs|------|- DDR controller
+          | | +----+      |
+  +----+  | |  |  |       |
+  |XTAL|--|-+  |  | +---+-|
+  +----+  | |  |  +-|AXI|-|- AXI-bus
+          | |  |    +---+-|
+          | |  |          |
+          | |  +----+---+-|- APB-bus
+          | +-------|SYS|-|- Low-speed Devices
+          |         +---+-|- High-speed Devices
+          +---------------+
+  Each CCU sub-block is represented as a separate dts-node and has an
+  individual driver to be bound with.
+
+  In order to create signals of wide range frequencies the external oscillator
+  output is primarily connected to a set of CCU PLLs. There are five PLLs
+  to create a clock for the MIPS P5600 cores, the embedded DDR controller,
+  SATA, Ethernet and PCIe domains. The last three domains though named by the
+  biggest system interfaces in fact include nearly all of the rest SoC
+  peripherals. Each of the PLLs is based on True Circuits TSMC CLN28HPM core
+  with an interface wrapper (so called safe PLL' clocks switcher) to simplify
+  the PLL configuration procedure. The PLLs work as depicted on the next
+  diagram:
+      +--------------------------+
+      |                          |
+      +-->+---+    +---+   +---+ |  +---+   0|\
+  CLKF--->|/NF|--->|PFD|...|VCO|-+->|/OD|--->| |
+          +---+ +->+---+   +---+ /->+---+    | |--->CLKOUT
+  CLKOD---------C----------------+          1| |
+       +--------C--------------------------->|/
+       |        |                             ^
+  Rclk-+->+---+ |                             |
+  CLKR--->|/NR|-+                             |
+          +---+                               |
+  BYPASS--------------------------------------+
+  BWADJ--->
+  where Rclk is the reference clock coming  from XTAL, NR - reference clock
+  divider, NF - PLL clock multiplier, OD - VCO output clock divider, CLKOUT -
+  output clock, BWADJ is the PLL bandwidth adjustment parameter. At this moment
+  the binding supports the PLL dividers configuration in accordance with a
+  requested rate, while bypassing and bandwidth adjustment settings can be
+  added in future if it gets to be necessary.
+
+  The PLLs CLKOUT is then either directly connected with the corresponding
+  clocks consumer (like P5600 cores or DDR controller) or passed over a CCU
+  divider to create a signal required for the clock domain.
+
+  The CCU PLL dts-node uses the common clock bindings [1] with no custom
+  parameters. The list of exported clocks can be found in
+  'dt-bindings/clock/bt1-ccu.h'.
+
+  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+allOf:
+  - $ref: /schemas/clock/clock.yaml#
+
+properties:
+  compatible:
+    const: be,bt1-ccu-pll
+
+  reg:
+    description: CCU PLLs sub-block base address.
+    maxItems: 1
+
+  "#clock-cells":
+    description: |
+      Clocks are referenced by the node phandle and an unique identifier
+      from 'dt-bindings/clock/bt1-ccu.h'.
+    const: 1
+
+  clocks:
+    description: Phandle of CCU External reference clock.
+    maxItems: 1
+
+  clock-names:
+    const: ref_clk
+
+  clock-output-names: true
+
+  assigned-clocks: true
+
+  assigned-clock-rates: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    ccu_pll: ccu_pll@1F04D000 {
+      compatible = "be,bt1-ccu-pll";
+      reg = <0x1F04D000 0x028>;
+      #clock-cells = <1>;
+
+      clocks = <&osc25>;
+      clock-names = "ref_clk";
+
+      clock-output-names = "cpu_pll", "sata_pll", "ddr_pll",
+                           "pcie_pll", "eth_pll";
+    };
+...
diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
new file mode 100644
index 000000000000..86e63162ade0
--- /dev/null
+++ b/include/dt-bindings/clock/bt1-ccu.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 BAIKAL ELECTRONICS, JSC
+ *
+ * Baikal-T1 CCU clock indeces.
+ */
+#ifndef __DT_BINDINGS_CLOCK_BT1_CCU_H
+#define __DT_BINDINGS_CLOCK_BT1_CCU_H
+
+/* Baikal-T1 CCU PLL indeces. */
+#define CCU_CPU_PLL			0
+#define CCU_SATA_PLL			1
+#define CCU_DDR_PLL			2
+#define CCU_PCIE_PLL			3
+#define CCU_ETH_PLL			4
+
+#endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
-- 
2.25.1

[PATCH 2/5] dt-bindings: clk: Add Baikal-T1 AXI-bus CCU bindings

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

After being gained by the CCU PLLs the signals must be transformed to
be suitable for the clock-consumers. This is done by a set of dividers
embedded into the CCU. A first block of dividers is used to create
reference clocks for AXI-bus of high-speed peripheral IP-cores of the
chip. So the AXI-bus CCU dts-node is an ordinary clock-provider with
standard set of properties supported. But in addition to that each
AXI-bus clock divider provide a way to reset the corresponding clock
domain. This makes the AXI-bus CCU dts-node to be also a reset-provider.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 .../bindings/clock/be,bt1-ccu-axi.yaml        | 151 ++++++++++++++++++
 include/dt-bindings/clock/bt1-ccu.h           |  13 ++
 include/dt-bindings/reset/bt1-ccu.h           |  23 +++
 3 files changed, 187 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
 create mode 100644 include/dt-bindings/reset/bt1-ccu.h

diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
new file mode 100644
index 000000000000..6b1eefdead27
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
@@ -0,0 +1,151 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
+#
+# Baikal-T1 AXI-bus Clocks Control Unit Device Tree Bindings.
+#
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/be,bt1-ccu-axi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Baikal-T1 AXI-bus Clock Control Unit
+
+maintainers:
+  - Serge Semin <fancer.lancer@gmail.com>
+
+description: |
+  Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
+  subsystems clocking and resetting. The CCU is connected with an external
+  fixed rate oscillator, which signal is transformed into clocks of various
+  frequencies and then propagated to either individual IP-blocks or to groups
+  of blocks (clock domains). The transformation is done by means of an embedded
+  into CCU PLLs and gateable/non-gateable dividers. Each clock domain can be
+  also individually reset by using the domain clocks divider configuration
+  registers. Baikal-T1 CCU is logically divided into the next components:
+  1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
+     in general can provide any frequency supported by the CCU PLLs).
+  2) PLLs clocks generators (PLLs).
+  3) AXI-bus clock dividers (AXI) - described in this bindings file.
+  4) System devices reference clock dividers (SYS).
+  which are connected with each other as shown on the next figure:
+          +---------------+
+          | Baikal-T1 CCU |
+          |   +----+------|- MIPS P5600 cores
+          | +-|PLLs|------|- DDR controller
+          | | +----+      |
+  +----+  | |  |  |       |
+  |XTAL|--|-+  |  | +---+-|
+  +----+  | |  |  +-|AXI|-|- AXI-bus
+          | |  |    +---+-|
+          | |  |          |
+          | |  +----+---+-|- APB-bus
+          | +-------|SYS|-|- Low-speed Devices
+          |         +---+-|- High-speed Devices
+          +---------------+
+  Each sub-block is represented as a separate dts-node and has an individual
+  driver to be bound with.
+
+  In order to create signals of wide range frequencies the external oscillator
+  output is primarily connected to a set of CCU PLLs. Some of PLLs CLKOUT are
+  then passed over CCU dividers to create signals required for the target clock
+  domain (like AXI-bus consumers). The dividers have the following structure:
+          +--------------+
+  CLKIN --|->+----+ 1|\  |
+  SETCLK--|--|/DIV|->| | |
+  CLKDIV--|--|    |  | |-|->CLKLOUT
+  LOCK----|--+----+  | | |
+          |          |/  |
+          |           |  |
+  EN------|-----------+  |
+  RST-----|--------------|->RSTOUT
+          +--------------+
+  where CLKIN is the reference clock coming either from a CCU PLL, SETCLK - a
+  command to update the output clock in accordance with a set divider,
+  CLKDIV - clocks divider, LOCK - a signal of the output clock stabilization,
+  EN - enable/disable the divider block, RST/RSTOUT - reset clocks domain
+  signal. Depending on the consumer IP-core peculiarities the dividers may lack
+  of some functionality depicted on the figure above (like EN,
+  CLKDIV/LOCK/SETCLK). In this case the corresponding clock provider just
+  doesn't expose either switching functions, or the rate configuration, or
+  both of them.
+
+  The CCU AXI dts-node uses the common clock bindings [1] with no custom
+  properties. The list of exported clocks and reset signals can be found in
+  the files: 'dt-bindings/clock/bt1-ccu.h' and 'dt-bindings/reset/bt1-ccu.h'.
+
+  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+allOf:
+  - $ref: /schemas/clock/clock.yaml#
+
+properties:
+  compatible:
+    const: be,bt1-ccu-axi
+
+  reg:
+    description: AXI-bus CCU dividers sub-block base address.
+    maxItems: 1
+
+  "#clock-cells":
+    description: |
+      Clocks are referenced by the node phandle and an unique identifier
+      from 'dt-bindings/clock/bt1-ccu.h'.
+    const: 1
+
+  "#reset-cells":
+    description: |
+      AXI-bus CCU sub-block provides a reset signal for each clock domain,
+      which unique identifiers are in 'dt-bindings/reset/bt1-ccu.h'.
+    const: 1
+
+  clocks:
+    items:
+      - description: CCU SATA PLL output clock.
+      - description: CCU PCIe PLL output clock.
+      - description: CCU Ethernet PLL output clock.
+
+  clock-names:
+    items:
+      - const: sata_clk
+      - const: pcie_clk
+      - const: eth_clk
+
+  clock-output-names: true
+
+  assigned-clocks: true
+
+  assigned-clock-rates: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    #include <dt-bindings/clock/bt1-ccu.h>
+
+    ccu_axi: ccu_axi@1F04D030 {
+      compatible = "be,bt1-ccu-axi";
+      reg = <0x1F04D030 0x030>;
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+
+      clocks = <&ccu_pll CCU_SATA_PLL>,
+               <&ccu_pll CCU_PCIE_PLL>,
+               <&ccu_pll CCU_ETH_PLL>;
+      clock-names = "sata_clk", "pcie_clk", "eth_clk";
+
+      clock-output-names = "axi_main_clk", "axi_ddr_clk",
+                           "axi_sata_clk", "axi_gmac0_clk",
+                           "axi_gmac1_clk", "axi_xgmac_clk",
+                           "axi_pcie_m_clk", "axi_pcie_s_clk",
+                           "axi_usb_clk", "axi_hwa_clk",
+                           "axi_sram_clk";
+    };
+...
diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
index 86e63162ade0..ebe723c6e0a8 100644
--- a/include/dt-bindings/clock/bt1-ccu.h
+++ b/include/dt-bindings/clock/bt1-ccu.h
@@ -14,4 +14,17 @@
 #define CCU_PCIE_PLL			3
 #define CCU_ETH_PLL			4
 
+/* Baikal-T1 AXI-bus CCU Clocks indeces. */
+#define CCU_AXI_MAIN_CLK		0
+#define CCU_AXI_DDR_CLK			1
+#define CCU_AXI_SATA_CLK		2
+#define CCU_AXI_GMAC0_CLK		3
+#define CCU_AXI_GMAC1_CLK		4
+#define CCU_AXI_XGMAC_CLK		5
+#define CCU_AXI_PCIE_M_CLK		6
+#define CCU_AXI_PCIE_S_CLK		7
+#define CCU_AXI_USB_CLK			8
+#define CCU_AXI_HWA_CLK			9
+#define CCU_AXI_SRAM_CLK		10
+
 #endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
diff --git a/include/dt-bindings/reset/bt1-ccu.h b/include/dt-bindings/reset/bt1-ccu.h
new file mode 100644
index 000000000000..4de5b6bcd433
--- /dev/null
+++ b/include/dt-bindings/reset/bt1-ccu.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 BAIKAL ELECTRONICS, JSC
+ *
+ * Baikal-T1 CCU reset indeces.
+ */
+#ifndef __DT_BINDINGS_RESET_BT1_CCU_H
+#define __DT_BINDINGS_RESET_BT1_CCU_H
+
+/* Baikal-T1 AXI-bus CCU Reset indeces. */
+#define CCU_AXI_MAIN_RST		0
+#define CCU_AXI_DDR_RST			1
+#define CCU_AXI_SATA_RST		2
+#define CCU_AXI_GMAC0_RST		3
+#define CCU_AXI_GMAC1_RST		4
+#define CCU_AXI_XGMAC_RST		5
+#define CCU_AXI_PCIE_M_RST		6
+#define CCU_AXI_PCIE_S_RST		7
+#define CCU_AXI_USB_RST			8
+#define CCU_AXI_HWA_RST			9
+#define CCU_AXI_SRAM_RST		10
+
+#endif /* __DT_BINDINGS_RESET_BT1_CCU_H */
-- 
2.25.1

[PATCH 3/5] dt-bindings: clk: Add Baikal-T1 System Devices CCU bindings

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Aside from providing an individual reference clocks for AXI-bus the
Baikal-T1 CCU dividers are used to alter the PLLs output signals for
the SoC peripheral devices. These dividers are represented by means
of the SYS CCU dts-node as an ordinary clock-provider with standard
set of properties supported. In the same way as AXI-bus CCU dividers
do they can be used to reset the APB and SATA clock domains, which
also makes the SYS CCU dts-node to be a reset-controller.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 .../bindings/clock/be,bt1-ccu-sys.yaml        | 169 ++++++++++++++++++
 include/dt-bindings/clock/bt1-ccu.h           |  24 +++
 include/dt-bindings/reset/bt1-ccu.h           |   4 +
 3 files changed, 197 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml

diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml
new file mode 100644
index 000000000000..aea09fbafc89
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml
@@ -0,0 +1,169 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
+#
+# Baikal-T1 System Devices Clocks Control Unit Device Tree Bindings.
+#
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/be,bt1-ccu-sys.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Baikal-T1 System Devices Clock Control Unit
+
+maintainers:
+  - Serge Semin <fancer.lancer@gmail.com>
+
+description: |
+  Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
+  subsystems clocking and resetting. The CCU is connected with an external
+  fixed rate oscillator, which signal is transformed into clocks of various
+  frequencies and then propagated to either individual IP-blocks or to groups
+  of blocks (clock domains). The transformation is done by means of an embedded
+  into CCU PLLs and gateable/non-gateable dividers. APB-bus divider register
+  provides a specific flag to initiate the full APB clock domain reset, so
+  causing each sub-device reset. Baikal-T1 CCU is logically divided into the
+  next components:
+  1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
+     in general can provide any frequency supported by the CCU PLLs).
+  2) PLLs clocks generators (PLLs).
+  3) AXI-bus clock dividers (AXI).
+  4) System devices reference clock dividers (SYS) - described in this bindings
+     file.
+  which are connected with each other as shown on the next figure:
+          +---------------+
+          | Baikal-T1 CCU |
+          |   +----+------|- MIPS P5600 cores
+          | +-|PLLs|------|- DDR controller
+          | | +----+      |
+  +----+  | |  |  |       |
+  |XTAL|--|-+  |  | +---+-|
+  +----+  | |  |  +-|AXI|-|- AXI-bus
+          | |  |    +---+-|
+          | |  |          |
+          | |  +----+---+-|- APB-bus
+          | +-------|SYS|-|- Low-speed Devices
+          |         +---+-|- High-speed Devices
+          +---------------+
+  Each sub-block is represented as a separate dts-node and has an individual
+  driver to be bound with.
+
+  In order to create signals of wide range frequencies the external oscillator
+  output is primarily connected to a set of CCU PLLs. The PLLs CLKOUT is then
+  either directly connected with the corresponding clocks consumer (like P5600
+  cores or DDR controller) or passed over a CCU divider to create a signal
+  required for the clock domain. The dividers have the following structure
+          +--------------+
+  CLKIN --|->+----+ 1|\  |
+  SETCLK--|--|/DIV|->| | |
+  CLKDIV--|--|    |  | |-|->CLKLOUT
+  LOCK----|--+----+  | | |
+          |          |/  |
+          |           |  |
+  EN------|-----------+  |
+  RST-----|--------------|->RSTOUT
+          +--------------+
+  where CLKIN is the reference clock coming either from XTAL or from a CCU PLL,
+  SETCLK - a command to update the output clock in accordance with a set
+  divider, CLKDIV - clocks divider, LOCK - a signal of the output clock
+  stabilization, EN - enable/disable the divider block, RST/RSTOUT - reset
+  clocks domain signal. Depending on the consumer IP-core peculiarities the
+  dividers may lack of some functionality depicted on the figure above (like
+  EN, CLKDIV/LOCK/SETCLK or RST). In this case the corresponding clock provider
+  just doesn't expose either switching functions, or the rate configuration, or
+  both of them.
+
+  The clock dividers, which output clock is then consumed by the SoC individual
+  devices, are united into a single clocks provider called System Devices CCU.
+  The System Devices CCU dts-node uses the common clock bindings [1] with no
+  custom properties. The list of exported clocks and reset signals can be found
+  in the files: 'dt-bindings/clock/bt1-ccu.h' and
+  'dt-bindings/reset/bt1-ccu.h'.
+
+  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+allOf:
+  - $ref: /schemas/clock/clock.yaml#
+
+properties:
+  compatible:
+    const: be,bt1-ccu-sys
+
+  reg:
+    items:
+      - description: System Devices CCU sub-block base address.
+      - description: Watchdog clock divider register address in CCU.
+
+  "#clock-cells":
+    description: |
+      Clocks are referenced by the node phandle and an unique identifier
+      from 'dt-bindings/clock/bt1-ccu.h'.
+    const: 1
+
+  "#reset-cells":
+    description: |
+      CCU system devices sub-block provides a reset signal for APB and SATA
+      clock domains, which unique identifiers reside in
+      'dt-bindings/reset/bt1-ccu.h'.
+    const: 1
+
+  clocks:
+    items:
+      - description: CCU external reference clock.
+      - description: CCU SATA PLL output clock.
+      - description: CCU PCIe PLL output clock.
+      - description: CCU Ethernet PLL output clock.
+
+  clock-names:
+    items:
+      - const: ref_clk
+      - const: sata_clk
+      - const: pcie_clk
+      - const: eth_clk
+
+  clock-output-names: true
+
+  assigned-clocks: true
+
+  assigned-clock-rates: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    #include <dt-bindings/clock/bt1-ccu.h>
+
+    ccu_sys: ccu_sys@1F04D060 {
+      compatible = "be,bt1-ccu-sys";
+      reg = <0x1F04D060 0x0A0>,
+            <0x1F04D150 0x004>;
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+
+      clocks = <&osc25>,
+               <&ccu_pll CCU_SATA_PLL>,
+               <&ccu_pll CCU_PCIE_PLL>,
+               <&ccu_pll CCU_ETH_PLL>;
+      clock-names = "ref_clk", "sata_clk", "pcie_clk",
+                    "eth_clk";
+
+      clock-output-names = "sys_sata_ref_clk", "sys_apb_clk",
+                           "sys_gmac0_csr_clk", "sys_gmac0_tx_clk",
+                           "sys_gmac0_ptp_clk", "sys_gmac1_csr_clk",
+                           "sys_gmac1_tx_clk", "sys_gmac1_ptp_clk",
+                           "sys_xgmac_ref_clk", "sys_xgmac_ptp_clk",
+                           "sys_usb_clk", "sys_pvt_clk",
+                           "sys_hwa_clk", "sys_uart_clk",
+                           "sys_spi_clk", "sys_i2c1_clk",
+                           "sys_i2c2_clk", "sys_gpio_clk",
+                           "sys_timer0_clk", "sys_timer1_clk",
+                           "sys_timer2_clk", "sys_wdt_clk";
+      };
+...
diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
index ebe723c6e0a8..4dcad1eb721f 100644
--- a/include/dt-bindings/clock/bt1-ccu.h
+++ b/include/dt-bindings/clock/bt1-ccu.h
@@ -27,4 +27,28 @@
 #define CCU_AXI_HWA_CLK			9
 #define CCU_AXI_SRAM_CLK		10
 
+/* Baikal-T1 System Devices CCU Clocks indeces. */
+#define CCU_SYS_SATA_REF_CLK		0
+#define CCU_SYS_APB_CLK			1
+#define CCU_SYS_GMAC0_CSR_CLK		2
+#define CCU_SYS_GMAC0_TX_CLK		3
+#define CCU_SYS_GMAC0_PTP_CLK		4
+#define CCU_SYS_GMAC1_CSR_CLK		5
+#define CCU_SYS_GMAC1_TX_CLK		6
+#define CCU_SYS_GMAC1_PTP_CLK		7
+#define CCU_SYS_XGMAC_REF_CLK		8
+#define CCU_SYS_XGMAC_PTP_CLK		9
+#define CCU_SYS_USB_CLK			10
+#define CCU_SYS_PVT_CLK			11
+#define CCU_SYS_HWA_CLK			12
+#define CCU_SYS_UART_CLK		13
+#define CCU_SYS_SPI_CLK			14
+#define CCU_SYS_I2C1_CLK		15
+#define CCU_SYS_I2C2_CLK		16
+#define CCU_SYS_GPIO_CLK		17
+#define CCU_SYS_TIMER0_CLK		18
+#define CCU_SYS_TIMER1_CLK		19
+#define CCU_SYS_TIMER2_CLK		20
+#define CCU_SYS_WDT_CLK			21
+
 #endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
diff --git a/include/dt-bindings/reset/bt1-ccu.h b/include/dt-bindings/reset/bt1-ccu.h
index 4de5b6bcd433..0bd8fd0edb41 100644
--- a/include/dt-bindings/reset/bt1-ccu.h
+++ b/include/dt-bindings/reset/bt1-ccu.h
@@ -20,4 +20,8 @@
 #define CCU_AXI_HWA_RST			9
 #define CCU_AXI_SRAM_RST		10
 
+/* Baikal-T1 System Devices CCU Reset indeces. */
+#define CCU_SYS_SATA_REF_RST		0
+#define CCU_SYS_APB_RST			1
+
 #endif /* __DT_BINDINGS_RESET_BT1_CCU_H */
-- 
2.25.1

Re: [PATCH 0/5] clk: Add Baikal-T1 SoC Clock Control Unit support

[Sergey Semin]

On Fri, Mar 06, 2020 at 04:00:43PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <fancer.lancer@gmail.com>
> 
> Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
> subsystems clocking and resetting. The CCU is connected with an external
> fixed rate oscillator, which signal is transformed into clocks of various
> frequencies and then propagated to either individual IP-blocks or to groups
> of blocks (clock domains). The transformation is done by means of PLLs and
> gateable/non-gateable, fixed/variable dividers embedded into the CCU. There
> are five PLLs to create a clock for the MIPS P5600 cores, the embedded DDR
> controller, SATA, Ethernet and PCIe domains. The last three PLLs CLKOUT are
> then passed over CCU dividers to create signals required for the target clock
> domains: individual AXI and APB bus clocks, SoC devices reference clocks.
> The CCU divider registers may also provide a way to reset the target devices
> state.
> 
> So this patchset introduces the Baikal-T1 clock and reset drivers of CCU
> PLLs, AXI-bus clock dividers and system devices clock dividers.
> 
> This patchset is rebased and tested on the mainline Linux kernel 5.6-rc4:
> commit 98d54f81e36b ("Linux 5.6-rc4").
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
> Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
> Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-clk@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Serge Semin (5):
>   dt-bindings: clk: Add Baikal-T1 CCU PLLs bindings
>   dt-bindings: clk: Add Baikal-T1 AXI-bus CCU bindings
>   dt-bindings: clk: Add Baikal-T1 System Devices CCU bindings
>   clk: Add Baikal-T1 CCU PLLs driver
>   clk: Add Baikal-T1 CCU dividers driver
> 
>  .../bindings/clock/be,bt1-ccu-axi.yaml        | 151 +++++
>  .../bindings/clock/be,bt1-ccu-pll.yaml        | 139 +++++
>  .../bindings/clock/be,bt1-ccu-sys.yaml        | 169 ++++++
>  drivers/clk/Kconfig                           |   1 +
>  drivers/clk/Makefile                          |   1 +
>  drivers/clk/baikal-t1/Kconfig                 |  46 ++
>  drivers/clk/baikal-t1/Makefile                |   3 +
>  drivers/clk/baikal-t1/ccu-div.c               | 531 ++++++++++++++++++
>  drivers/clk/baikal-t1/ccu-div.h               | 114 ++++
>  drivers/clk/baikal-t1/ccu-pll.c               | 474 ++++++++++++++++
>  drivers/clk/baikal-t1/ccu-pll.h               |  73 +++
>  drivers/clk/baikal-t1/clk-ccu-div.c           | 522 +++++++++++++++++
>  drivers/clk/baikal-t1/clk-ccu-pll.c           | 217 +++++++
>  drivers/clk/baikal-t1/common.h                |  51 ++
>  include/dt-bindings/clock/bt1-ccu.h           |  54 ++
>  include/dt-bindings/reset/bt1-ccu.h           |  27 +
>  16 files changed, 2573 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml
>  create mode 100644 drivers/clk/baikal-t1/Kconfig
>  create mode 100644 drivers/clk/baikal-t1/Makefile
>  create mode 100644 drivers/clk/baikal-t1/ccu-div.c
>  create mode 100644 drivers/clk/baikal-t1/ccu-div.h
>  create mode 100644 drivers/clk/baikal-t1/ccu-pll.c
>  create mode 100644 drivers/clk/baikal-t1/ccu-pll.h
>  create mode 100644 drivers/clk/baikal-t1/clk-ccu-div.c
>  create mode 100644 drivers/clk/baikal-t1/clk-ccu-pll.c
>  create mode 100644 drivers/clk/baikal-t1/common.h
>  create mode 100644 include/dt-bindings/clock/bt1-ccu.h
>  create mode 100644 include/dt-bindings/reset/bt1-ccu.h
> 
> -- 
> 2.25.1
> 

Folks,

It appears our corporate email server changes the Message-Id field of 
messages passing through it. Due to that the emails threading gets to be
broken. I'll resubmit the properly structured patchset as soon as our system
administrator fixes the problem. Sorry for the inconvenience caused by it.

Regards,
-Sergey

Re: [PATCH 1/5] dt-bindings: clk: Add Baikal-T1 CCU PLLs bindings

[Stephen Boyd]

Quoting Sergey.Semin@baikalelectronics.ru (2020-03-06 05:00:44)
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Baikal-T1 Clocks Control Unit is responsible for transformation of a
> signal coming from an external oscillator into clocks of various
> frequencies to propagate them then to the corresponding clocks
> consumers (either individual IP-blocks or clock domains). In order
> to create a set of high-frequency clocks the external signal is
> firstly handled by the embedded into CCU PLLs. So the corresponding
> dts-node is just a normal clock-provider node with standard set of
> properties.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>

SoB chain is backwards. Is Alexey the author? Or Co-developed-by?

> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---
>  .../bindings/clock/be,bt1-ccu-pll.yaml        | 139 ++++++++++++++++++
>  include/dt-bindings/clock/bt1-ccu.h           |  17 +++
>  2 files changed, 156 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
>  create mode 100644 include/dt-bindings/clock/bt1-ccu.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> new file mode 100644
> index 000000000000..f2e397cc147b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> @@ -0,0 +1,139 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
> +#
> +# Baikal-T1 Clocks Control Unit PLL Device Tree Bindings.
> +#

I don't think we need any of these comments besides the license
identifier line. Can you dual license this?

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/be,bt1-ccu-pll.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Baikal-T1 Clock Control Unit PLLs
> +
> +maintainers:
> +  - Serge Semin <fancer.lancer@gmail.com>
> +
> +description: |
> +  Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
> +  subsystems clocking and resetting. The CCU is connected with an external
> +  fixed rate oscillator, which signal is transformed into clocks of various
> +  frequencies and then propagated to either individual IP-blocks or to groups
> +  of blocks (clock domains). The transformation is done by means of PLLs and
> +  gateable/non-gateable dividers embedded into the CCU. It's logically divided
> +  into the next components:
> +  1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
> +     in general can provide any frequency supported by the CCU PLLs).
> +  2) PLLs clocks generators (PLLs) - described in this bindings file.
> +  3) AXI-bus clock dividers (AXI).
> +  4) System devices reference clock dividers (SYS).
> +  which are connected with each other as shown on the next figure:

Please add a newline here

> +          +---------------+
> +          | Baikal-T1 CCU |
> +          |   +----+------|- MIPS P5600 cores
> +          | +-|PLLs|------|- DDR controller
> +          | | +----+      |
> +  +----+  | |  |  |       |
> +  |XTAL|--|-+  |  | +---+-|
> +  +----+  | |  |  +-|AXI|-|- AXI-bus
> +          | |  |    +---+-|
> +          | |  |          |
> +          | |  +----+---+-|- APB-bus
> +          | +-------|SYS|-|- Low-speed Devices
> +          |         +---+-|- High-speed Devices
> +          +---------------+

And here.

> +  Each CCU sub-block is represented as a separate dts-node and has an
> +  individual driver to be bound with.
> +
> +  In order to create signals of wide range frequencies the external oscillator
> +  output is primarily connected to a set of CCU PLLs. There are five PLLs
> +  to create a clock for the MIPS P5600 cores, the embedded DDR controller,
> +  SATA, Ethernet and PCIe domains. The last three domains though named by the
> +  biggest system interfaces in fact include nearly all of the rest SoC
> +  peripherals. Each of the PLLs is based on True Circuits TSMC CLN28HPM core
> +  with an interface wrapper (so called safe PLL' clocks switcher) to simplify
> +  the PLL configuration procedure. The PLLs work as depicted on the next
> +  diagram:

Same, space out the diagrams.

> +      +--------------------------+
> +      |                          |
> +      +-->+---+    +---+   +---+ |  +---+   0|\
> +  CLKF--->|/NF|--->|PFD|...|VCO|-+->|/OD|--->| |
> +          +---+ +->+---+   +---+ /->+---+    | |--->CLKOUT
> +  CLKOD---------C----------------+          1| |
> +       +--------C--------------------------->|/
> +       |        |                             ^
> +  Rclk-+->+---+ |                             |
> +  CLKR--->|/NR|-+                             |
> +          +---+                               |
> +  BYPASS--------------------------------------+
> +  BWADJ--->
> +  where Rclk is the reference clock coming  from XTAL, NR - reference clock
> +  divider, NF - PLL clock multiplier, OD - VCO output clock divider, CLKOUT -
> +  output clock, BWADJ is the PLL bandwidth adjustment parameter. At this moment
> +  the binding supports the PLL dividers configuration in accordance with a
> +  requested rate, while bypassing and bandwidth adjustment settings can be
> +  added in future if it gets to be necessary.
> +
> +  The PLLs CLKOUT is then either directly connected with the corresponding
> +  clocks consumer (like P5600 cores or DDR controller) or passed over a CCU
> +  divider to create a signal required for the clock domain.
> +
> +  The CCU PLL dts-node uses the common clock bindings [1] with no custom
> +  parameters. The list of exported clocks can be found in
> +  'dt-bindings/clock/bt1-ccu.h'.
> +
> +  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt

Don't think we need to mention this binding anymore. But it's good that
we know what exported clock ids are.

> +
> +allOf:
> +  - $ref: /schemas/clock/clock.yaml#
> +
> +properties:
> +  compatible:
> +    const: be,bt1-ccu-pll
> +
> +  reg:
> +    description: CCU PLLs sub-block base address.
> +    maxItems: 1
> +
> +  "#clock-cells":
> +    description: |
> +      Clocks are referenced by the node phandle and an unique identifier
> +      from 'dt-bindings/clock/bt1-ccu.h'.

Don't think we need this description.

> +    const: 1
> +
> +  clocks:
> +    description: Phandle of CCU External reference clock.
> +    maxItems: 1
> +
> +  clock-names:
> +    const: ref_clk

Can we drop _clk? It's redundant.

> +
> +  clock-output-names: true
> +
> +  assigned-clocks: true
> +
> +  assigned-clock-rates: true
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +  - clocks
> +  - clock-names
> +
> +examples:
> +  - |
> +    ccu_pll: ccu_pll@1F04D000 {

Drop the phandle unless it's actually used.

> +      compatible = "be,bt1-ccu-pll";
> +      reg = <0x1F04D000 0x028>;

Lowercase hex please. That size is oddly small.

> +      #clock-cells = <1>;
> +
> +      clocks = <&osc25>;
> +      clock-names = "ref_clk";
> +
> +      clock-output-names = "cpu_pll", "sata_pll", "ddr_pll",
> +                           "pcie_pll", "eth_pll";
> +    };
> +...
> diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
> new file mode 100644
> index 000000000000..86e63162ade0
> --- /dev/null
> +++ b/include/dt-bindings/clock/bt1-ccu.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019 BAIKAL ELECTRONICS, JSC
> + *
> + * Baikal-T1 CCU clock indeces.
> + */
> +#ifndef __DT_BINDINGS_CLOCK_BT1_CCU_H
> +#define __DT_BINDINGS_CLOCK_BT1_CCU_H
> +
> +/* Baikal-T1 CCU PLL indeces. */

Please drop this comment. It's not useful.

> +#define CCU_CPU_PLL                    0
> +#define CCU_SATA_PLL                   1
> +#define CCU_DDR_PLL                    2
> +#define CCU_PCIE_PLL                   3
> +#define CCU_ETH_PLL                    4
> +
> +#endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
> -- 
> 2.25.1
>

Re: [PATCH 0/5] clk: Add Baikal-T1 SoC Clock Control Unit support

[Stephen Boyd]

Quoting Sergey Semin (2020-03-09 17:21:26)
> 
> It appears our corporate email server changes the Message-Id field of 
> messages passing through it. Due to that the emails threading gets to be
> broken. I'll resubmit the properly structured patchset as soon as our system
> administrator fixes the problem. Sorry for the inconvenience caused by it.
> 

Please trim replies. I had to scroll and that made my life super hard! :P 

Anyway, I see a thread so maybe my MUA figured it out. I can wait for it
to be sorted on the corporate end though.

Re: [PATCH 3/5] dt-bindings: clk: Add Baikal-T1 System Devices CCU bindings

[Stephen Boyd]

Quoting Sergey.Semin@baikalelectronics.ru (2020-03-06 05:00:46)
> diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml
> new file mode 100644
> index 000000000000..aea09fbafc89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml
> @@ -0,0 +1,169 @@
[..]
> +  assigned-clock-rates: true
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +  - clocks
> +  - clock-names
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/bt1-ccu.h>
> +
> +    ccu_sys: ccu_sys@1F04D060 {

Node name should be clock-controller@1f04d060.

Also, binding looks wrong because that address isn't aligned. Most
likely it's one hardware block that has many different functionalities
so splitting it up into different regions isn't doing anything besides
logically splitting up the register space for software benefits.

> +      compatible = "be,bt1-ccu-sys";
> +      reg = <0x1F04D060 0x0A0>,
> +            <0x1F04D150 0x004>;
> +      #clock-cells = <1>;
> +      #reset-cells = <1>;
> +
> +      clocks = <&osc25>,
> +               <&ccu_pll CCU_SATA_PLL>,
> +               <&ccu_pll CCU_PCIE_PLL>,
> +               <&ccu_pll CCU_ETH_PLL>;
> +      clock-names = "ref_clk", "sata_clk", "pcie_clk",
> +                    "eth_clk";
> +
> +      clock-output-names = "sys_sata_ref_clk", "sys_apb_clk",
> +                           "sys_gmac0_csr_clk", "sys_gmac0_tx_clk",
> +                           "sys_gmac0_ptp_clk", "sys_gmac1_csr_clk",
> +                           "sys_gmac1_tx_clk", "sys_gmac1_ptp_clk",
> +                           "sys_xgmac_ref_clk", "sys_xgmac_ptp_clk",
> +                           "sys_usb_clk", "sys_pvt_clk",
> +                           "sys_hwa_clk", "sys_uart_clk",
> +                           "sys_spi_clk", "sys_i2c1_clk",
> +                           "sys_i2c2_clk", "sys_gpio_clk",
> +                           "sys_timer0_clk", "sys_timer1_clk",
> +                           "sys_timer2_clk", "sys_wdt_clk";
> +      };
> +...
> diff --git a/include/dt-bindings/reset/bt1-ccu.h b/include/dt-bindings/reset/bt1-ccu.h
> index 4de5b6bcd433..0bd8fd0edb41 100644
> --- a/include/dt-bindings/reset/bt1-ccu.h
> +++ b/include/dt-bindings/reset/bt1-ccu.h
> @@ -20,4 +20,8 @@
>  #define CCU_AXI_HWA_RST                        9
>  #define CCU_AXI_SRAM_RST               10
>  
> +/* Baikal-T1 System Devices CCU Reset indeces. */

indeces is not a word.

> +#define CCU_SYS_SATA_REF_RST           0
> +#define CCU_SYS_APB_RST                        1
> +

Re: [PATCH 2/5] dt-bindings: clk: Add Baikal-T1 AXI-bus CCU bindings

[Rob Herring]

On Fri, Mar 06, 2020 at 04:00:45PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> After being gained by the CCU PLLs the signals must be transformed to
> be suitable for the clock-consumers. This is done by a set of dividers
> embedded into the CCU. A first block of dividers is used to create
> reference clocks for AXI-bus of high-speed peripheral IP-cores of the
> chip. So the AXI-bus CCU dts-node is an ordinary clock-provider with
> standard set of properties supported. But in addition to that each
> AXI-bus clock divider provide a way to reset the corresponding clock
> domain. This makes the AXI-bus CCU dts-node to be also a reset-provider.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---
>  .../bindings/clock/be,bt1-ccu-axi.yaml        | 151 ++++++++++++++++++
>  include/dt-bindings/clock/bt1-ccu.h           |  13 ++
>  include/dt-bindings/reset/bt1-ccu.h           |  23 +++
>  3 files changed, 187 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
>  create mode 100644 include/dt-bindings/reset/bt1-ccu.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
> new file mode 100644
> index 000000000000..6b1eefdead27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
> @@ -0,0 +1,151 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings:

(GPL-2.0-only OR BSD-2-Clause)

> +#
> +# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
> +#
> +# Baikal-T1 AXI-bus Clocks Control Unit Device Tree Bindings.

As Stephen said, drop this. You can keep the copyright, just make it 
line 2.

> +#
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/be,bt1-ccu-axi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Baikal-T1 AXI-bus Clock Control Unit
> +
> +maintainers:
> +  - Serge Semin <fancer.lancer@gmail.com>
> +
> +description: |
> +  Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
> +  subsystems clocking and resetting. The CCU is connected with an external
> +  fixed rate oscillator, which signal is transformed into clocks of various
> +  frequencies and then propagated to either individual IP-blocks or to groups
> +  of blocks (clock domains). The transformation is done by means of an embedded
> +  into CCU PLLs and gateable/non-gateable dividers. Each clock domain can be
> +  also individually reset by using the domain clocks divider configuration
> +  registers. Baikal-T1 CCU is logically divided into the next components:
> +  1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
> +     in general can provide any frequency supported by the CCU PLLs).
> +  2) PLLs clocks generators (PLLs).
> +  3) AXI-bus clock dividers (AXI) - described in this bindings file.
> +  4) System devices reference clock dividers (SYS).
> +  which are connected with each other as shown on the next figure:
> +          +---------------+
> +          | Baikal-T1 CCU |
> +          |   +----+------|- MIPS P5600 cores
> +          | +-|PLLs|------|- DDR controller
> +          | | +----+      |
> +  +----+  | |  |  |       |
> +  |XTAL|--|-+  |  | +---+-|
> +  +----+  | |  |  +-|AXI|-|- AXI-bus
> +          | |  |    +---+-|
> +          | |  |          |
> +          | |  +----+---+-|- APB-bus
> +          | +-------|SYS|-|- Low-speed Devices
> +          |         +---+-|- High-speed Devices
> +          +---------------+
> +  Each sub-block is represented as a separate dts-node and has an individual
> +  driver to be bound with.
> +
> +  In order to create signals of wide range frequencies the external oscillator
> +  output is primarily connected to a set of CCU PLLs. Some of PLLs CLKOUT are
> +  then passed over CCU dividers to create signals required for the target clock
> +  domain (like AXI-bus consumers). The dividers have the following structure:
> +          +--------------+
> +  CLKIN --|->+----+ 1|\  |
> +  SETCLK--|--|/DIV|->| | |
> +  CLKDIV--|--|    |  | |-|->CLKLOUT
> +  LOCK----|--+----+  | | |
> +          |          |/  |
> +          |           |  |
> +  EN------|-----------+  |
> +  RST-----|--------------|->RSTOUT
> +          +--------------+
> +  where CLKIN is the reference clock coming either from a CCU PLL, SETCLK - a
> +  command to update the output clock in accordance with a set divider,
> +  CLKDIV - clocks divider, LOCK - a signal of the output clock stabilization,
> +  EN - enable/disable the divider block, RST/RSTOUT - reset clocks domain
> +  signal. Depending on the consumer IP-core peculiarities the dividers may lack
> +  of some functionality depicted on the figure above (like EN,
> +  CLKDIV/LOCK/SETCLK). In this case the corresponding clock provider just
> +  doesn't expose either switching functions, or the rate configuration, or
> +  both of them.
> +
> +  The CCU AXI dts-node uses the common clock bindings [1] with no custom
> +  properties. The list of exported clocks and reset signals can be found in
> +  the files: 'dt-bindings/clock/bt1-ccu.h' and 'dt-bindings/reset/bt1-ccu.h'.
> +
> +  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +allOf:
> +  - $ref: /schemas/clock/clock.yaml#

Drop this, not needed (clock.yaml is applied to every node).

> +
> +properties:
> +  compatible:
> +    const: be,bt1-ccu-axi
> +
> +  reg:
> +    description: AXI-bus CCU dividers sub-block base address.

Don't really need this.

> +    maxItems: 1
> +
> +  "#clock-cells":
> +    description: |
> +      Clocks are referenced by the node phandle and an unique identifier
> +      from 'dt-bindings/clock/bt1-ccu.h'.
> +    const: 1
> +
> +  "#reset-cells":
> +    description: |
> +      AXI-bus CCU sub-block provides a reset signal for each clock domain,
> +      which unique identifiers are in 'dt-bindings/reset/bt1-ccu.h'.
> +    const: 1
> +
> +  clocks:
> +    items:
> +      - description: CCU SATA PLL output clock.
> +      - description: CCU PCIe PLL output clock.
> +      - description: CCU Ethernet PLL output clock.
> +
> +  clock-names:
> +    items:
> +      - const: sata_clk
> +      - const: pcie_clk
> +      - const: eth_clk
> +
> +  clock-output-names: true
> +
> +  assigned-clocks: true
> +
> +  assigned-clock-rates: true
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +  - clocks
> +  - clock-names
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/bt1-ccu.h>
> +
> +    ccu_axi: ccu_axi@1F04D030 {

clock-controller@1f04d030

> +      compatible = "be,bt1-ccu-axi";
> +      reg = <0x1F04D030 0x030>;
> +      #clock-cells = <1>;
> +      #reset-cells = <1>;
> +
> +      clocks = <&ccu_pll CCU_SATA_PLL>,
> +               <&ccu_pll CCU_PCIE_PLL>,
> +               <&ccu_pll CCU_ETH_PLL>;
> +      clock-names = "sata_clk", "pcie_clk", "eth_clk";
> +
> +      clock-output-names = "axi_main_clk", "axi_ddr_clk",
> +                           "axi_sata_clk", "axi_gmac0_clk",
> +                           "axi_gmac1_clk", "axi_xgmac_clk",
> +                           "axi_pcie_m_clk", "axi_pcie_s_clk",
> +                           "axi_usb_clk", "axi_hwa_clk",
> +                           "axi_sram_clk";
> +    };
> +...
> diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
> index 86e63162ade0..ebe723c6e0a8 100644
> --- a/include/dt-bindings/clock/bt1-ccu.h
> +++ b/include/dt-bindings/clock/bt1-ccu.h
> @@ -14,4 +14,17 @@
>  #define CCU_PCIE_PLL			3
>  #define CCU_ETH_PLL			4
>  
> +/* Baikal-T1 AXI-bus CCU Clocks indeces. */
> +#define CCU_AXI_MAIN_CLK		0
> +#define CCU_AXI_DDR_CLK			1
> +#define CCU_AXI_SATA_CLK		2
> +#define CCU_AXI_GMAC0_CLK		3
> +#define CCU_AXI_GMAC1_CLK		4
> +#define CCU_AXI_XGMAC_CLK		5
> +#define CCU_AXI_PCIE_M_CLK		6
> +#define CCU_AXI_PCIE_S_CLK		7
> +#define CCU_AXI_USB_CLK			8
> +#define CCU_AXI_HWA_CLK			9
> +#define CCU_AXI_SRAM_CLK		10
> +
>  #endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
> diff --git a/include/dt-bindings/reset/bt1-ccu.h b/include/dt-bindings/reset/bt1-ccu.h
> new file mode 100644
> index 000000000000..4de5b6bcd433
> --- /dev/null
> +++ b/include/dt-bindings/reset/bt1-ccu.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019 BAIKAL ELECTRONICS, JSC
> + *
> + * Baikal-T1 CCU reset indeces.
> + */
> +#ifndef __DT_BINDINGS_RESET_BT1_CCU_H
> +#define __DT_BINDINGS_RESET_BT1_CCU_H
> +
> +/* Baikal-T1 AXI-bus CCU Reset indeces. */
> +#define CCU_AXI_MAIN_RST		0
> +#define CCU_AXI_DDR_RST			1
> +#define CCU_AXI_SATA_RST		2
> +#define CCU_AXI_GMAC0_RST		3
> +#define CCU_AXI_GMAC1_RST		4
> +#define CCU_AXI_XGMAC_RST		5
> +#define CCU_AXI_PCIE_M_RST		6
> +#define CCU_AXI_PCIE_S_RST		7
> +#define CCU_AXI_USB_RST			8
> +#define CCU_AXI_HWA_RST			9
> +#define CCU_AXI_SRAM_RST		10
> +
> +#endif /* __DT_BINDINGS_RESET_BT1_CCU_H */
> -- 
> 2.25.1
> 

Re: [PATCH 1/5] dt-bindings: clk: Add Baikal-T1 CCU PLLs bindings

[Sergey Semin]

Hello Stephen,

Sorry for a delayed response. My answers to your comments are below.

On Mon, Mar 09, 2020 at 07:02:27PM -0700, Stephen Boyd wrote:
> Quoting Sergey.Semin@baikalelectronics.ru (2020-03-06 05:00:44)
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > Baikal-T1 Clocks Control Unit is responsible for transformation of a
> > signal coming from an external oscillator into clocks of various
> > frequencies to propagate them then to the corresponding clocks
> > consumers (either individual IP-blocks or clock domains). In order
> > to create a set of high-frequency clocks the external signal is
> > firstly handled by the embedded into CCU PLLs. So the corresponding
> > dts-node is just a normal clock-provider node with standard set of
> > properties.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> 
> SoB chain is backwards. Is Alexey the author? Or Co-developed-by?

Thanks for noticing this. I'll rearrange the SoB's in v2.

> 
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > ---
> >  .../bindings/clock/be,bt1-ccu-pll.yaml        | 139 ++++++++++++++++++
> >  include/dt-bindings/clock/bt1-ccu.h           |  17 +++
> >  2 files changed, 156 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> >  create mode 100644 include/dt-bindings/clock/bt1-ccu.h
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> > new file mode 100644
> > index 000000000000..f2e397cc147b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> > @@ -0,0 +1,139 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
> > +#
> > +# Baikal-T1 Clocks Control Unit PLL Device Tree Bindings.
> > +#
> 
> I don't think we need any of these comments besides the license
> identifier line. Can you dual license this?
> 

It's normal to have a copyright here, but in a single-lined form.
I'll do this in v2 and also dual license the binding file.

> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/be,bt1-ccu-pll.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Baikal-T1 Clock Control Unit PLLs
> > +
> > +maintainers:
> > +  - Serge Semin <fancer.lancer@gmail.com>
> > +
> > +description: |
> > +  Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
> > +  subsystems clocking and resetting. The CCU is connected with an external
> > +  fixed rate oscillator, which signal is transformed into clocks of various
> > +  frequencies and then propagated to either individual IP-blocks or to groups
> > +  of blocks (clock domains). The transformation is done by means of PLLs and
> > +  gateable/non-gateable dividers embedded into the CCU. It's logically divided
> > +  into the next components:
> > +  1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
> > +     in general can provide any frequency supported by the CCU PLLs).
> > +  2) PLLs clocks generators (PLLs) - described in this bindings file.
> > +  3) AXI-bus clock dividers (AXI).
> > +  4) System devices reference clock dividers (SYS).
> > +  which are connected with each other as shown on the next figure:
> 
> Please add a newline here

Ok.

> 
> > +          +---------------+
> > +          | Baikal-T1 CCU |
> > +          |   +----+------|- MIPS P5600 cores
> > +          | +-|PLLs|------|- DDR controller
> > +          | | +----+      |
> > +  +----+  | |  |  |       |
> > +  |XTAL|--|-+  |  | +---+-|
> > +  +----+  | |  |  +-|AXI|-|- AXI-bus
> > +          | |  |    +---+-|
> > +          | |  |          |
> > +          | |  +----+---+-|- APB-bus
> > +          | +-------|SYS|-|- Low-speed Devices
> > +          |         +---+-|- High-speed Devices
> > +          +---------------+
> 
> And here.
> 

Ok

> > +  Each CCU sub-block is represented as a separate dts-node and has an
> > +  individual driver to be bound with.
> > +
> > +  In order to create signals of wide range frequencies the external oscillator
> > +  output is primarily connected to a set of CCU PLLs. There are five PLLs
> > +  to create a clock for the MIPS P5600 cores, the embedded DDR controller,
> > +  SATA, Ethernet and PCIe domains. The last three domains though named by the
> > +  biggest system interfaces in fact include nearly all of the rest SoC
> > +  peripherals. Each of the PLLs is based on True Circuits TSMC CLN28HPM core
> > +  with an interface wrapper (so called safe PLL' clocks switcher) to simplify
> > +  the PLL configuration procedure. The PLLs work as depicted on the next
> > +  diagram:
> 
> Same, space out the diagrams.
> 

Ok

> > +      +--------------------------+
> > +      |                          |
> > +      +-->+---+    +---+   +---+ |  +---+   0|\
> > +  CLKF--->|/NF|--->|PFD|...|VCO|-+->|/OD|--->| |
> > +          +---+ +->+---+   +---+ /->+---+    | |--->CLKOUT
> > +  CLKOD---------C----------------+          1| |
> > +       +--------C--------------------------->|/
> > +       |        |                             ^
> > +  Rclk-+->+---+ |                             |
> > +  CLKR--->|/NR|-+                             |
> > +          +---+                               |
> > +  BYPASS--------------------------------------+
> > +  BWADJ--->
> > +  where Rclk is the reference clock coming  from XTAL, NR - reference clock
> > +  divider, NF - PLL clock multiplier, OD - VCO output clock divider, CLKOUT -
> > +  output clock, BWADJ is the PLL bandwidth adjustment parameter. At this moment
> > +  the binding supports the PLL dividers configuration in accordance with a
> > +  requested rate, while bypassing and bandwidth adjustment settings can be
> > +  added in future if it gets to be necessary.
> > +
> > +  The PLLs CLKOUT is then either directly connected with the corresponding
> > +  clocks consumer (like P5600 cores or DDR controller) or passed over a CCU
> > +  divider to create a signal required for the clock domain.
> > +
> > +  The CCU PLL dts-node uses the common clock bindings [1] with no custom
> > +  parameters. The list of exported clocks can be found in
> > +  'dt-bindings/clock/bt1-ccu.h'.
> > +
> > +  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> 
> Don't think we need to mention this binding anymore. But it's good that
> we know what exported clock ids are.
> 

Ok. I'll remove the legacy text binding file mention here and retain the
reference to the header file with the clock IDs defined. The similar
thing will be done for the others bindings in the patchset.

> > +
> > +allOf:
> > +  - $ref: /schemas/clock/clock.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: be,bt1-ccu-pll
> > +

> > +  reg:
> > +    description: CCU PLLs sub-block base address.
> > +    maxItems: 1
> > +

Sometime ago I sent a RFC to Rob and you being in Cc there:
https://lkml.org/lkml/2020/3/22/393
Simply speaking there are several issues raised in comments to different
patchsets, which are indirectly connected with the Baikal-T1 System Controller
DT node design I've initially chosen. In accordance with that I've spread its
functional blocks into different DT nodes with no reference to being related
to the System Controller. Clock Control Unit nodes are amongst these blocks.
Seeing such design caused these issues I suggested an alternative solution
of having a single System Controller node and multiple functional sub-nodes.
These sub-nodes will include the Clock Control Unit PLLs, AXI-bus and System
Device blocks. I thoroughly described the solution in the RFC. So if no
arguments against it pop up soon in the RFC comments, I'll implement it in
v2 of this patchset as well. This solution cause the reg-property removal
from this binding. Instead the drivers shall refer to the parental syscon
node to get a regmap with CCU registers from it.

> > +  "#clock-cells":
> > +    description: |
> > +      Clocks are referenced by the node phandle and an unique identifier
> > +      from 'dt-bindings/clock/bt1-ccu.h'.
> 
> Don't think we need this description.

Agreed.

> 
> > +    const: 1
> > +
> > +  clocks:
> > +    description: Phandle of CCU External reference clock.
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: ref_clk
> 
> Can we drop _clk? It's redundant.

I would leave this and "pcie_clk", "sata_clk", "eth_clk" declared in the
next two bindings as is, since this way they would exactly match the names
used in the documentation. The same thing is with the clock-output-names
property values.

I've seen such names in many other drivers/bindings including the
bindings in the clock subsystem even submitted rather recently, not to
mention the names like "aclk", "pclk", etc used all over the dt nodes.
Are there any requirements in naming the clocks? Should I avoid using the
'_clk' clock names suffix in accordance with them? If so, please point
me out to that requirement in docs for future reference.

Normally If I don't find something in the requirements documented in the kernel,
I use either a commonly utilized practice seen in other similar drivers, or
select a solution which seems better to me like providing a better readability
and code understanding. 

> 
> > +
> > +  clock-output-names: true
> > +
> > +  assigned-clocks: true
> > +
> > +  assigned-clock-rates: true
> > +
> > +additionalProperties: false
> > +

I'll also replace these four properties with a single
"unevaluatedProperties: false". In the framework of other patchset
review Rob said this property is more suitable in such situations and
will get to be supported by the dt_binding_check script eventually.

> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#clock-cells"
> > +  - clocks
> > +  - clock-names
> > +
> > +examples:
> > +  - |
> > +    ccu_pll: ccu_pll@1F04D000 {
> 
> Drop the phandle unless it's actually used.

Do you mean the label definition? If so, Ok. I'll remove it.

Unit-address will be also lowercased if I don't remove the reg property
from here. As I said in RFC in accordance with the alternative solution
this node will be a sub-node of the system controller, which regmap will
be used instead of the individual reg-property definition. So if the
reg-property is removed from the node, the unit-address will be also
discarded from here.

> 
> > +      compatible = "be,bt1-ccu-pll";
> > +      reg = <0x1F04D000 0x028>;
> 
> Lowercase hex please. That size is oddly small.

It's small due to be range being part of the system controller registers
set. I've briefly described this above and thoroughly - in the RFC.
Please see the RFC text and send your comments regarding an alternative
solution there shall you have any.

Anyway if no comments are received there soon, I'll remove the reg
property from here. The PLL driver will refer to the parental system
controller to get the registers regmap handler.

> 
> > +      #clock-cells = <1>;
> > +
> > +      clocks = <&osc25>;
> > +      clock-names = "ref_clk";
> > +
> > +      clock-output-names = "cpu_pll", "sata_pll", "ddr_pll",
> > +                           "pcie_pll", "eth_pll";
> > +    };
> > +...
> > diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
> > new file mode 100644
> > index 000000000000..86e63162ade0
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/bt1-ccu.h
> > @@ -0,0 +1,17 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2019 BAIKAL ELECTRONICS, JSC
> > + *
> > + * Baikal-T1 CCU clock indeces.
> > + */
> > +#ifndef __DT_BINDINGS_CLOCK_BT1_CCU_H
> > +#define __DT_BINDINGS_CLOCK_BT1_CCU_H
> > +
> > +/* Baikal-T1 CCU PLL indeces. */
> 
> Please drop this comment. It's not useful.

Ok.

Regards,
-Sergey

> 
> > +#define CCU_CPU_PLL                    0
> > +#define CCU_SATA_PLL                   1
> > +#define CCU_DDR_PLL                    2
> > +#define CCU_PCIE_PLL                   3
> > +#define CCU_ETH_PLL                    4
> > +
> > +#endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
> > -- 
> > 2.25.1
> >

Re: [PATCH 2/5] dt-bindings: clk: Add Baikal-T1 AXI-bus CCU bindings

[Sergey Semin]

On Thu, Mar 12, 2020 at 03:50:11PM -0500, Rob Herring wrote:
> On Fri, Mar 06, 2020 at 04:00:45PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > After being gained by the CCU PLLs the signals must be transformed to
> > be suitable for the clock-consumers. This is done by a set of dividers
> > embedded into the CCU. A first block of dividers is used to create
> > reference clocks for AXI-bus of high-speed peripheral IP-cores of the
> > chip. So the AXI-bus CCU dts-node is an ordinary clock-provider with
> > standard set of properties supported. But in addition to that each
> > AXI-bus clock divider provide a way to reset the corresponding clock
> > domain. This makes the AXI-bus CCU dts-node to be also a reset-provider.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > ---
> >  .../bindings/clock/be,bt1-ccu-axi.yaml        | 151 ++++++++++++++++++
> >  include/dt-bindings/clock/bt1-ccu.h           |  13 ++
> >  include/dt-bindings/reset/bt1-ccu.h           |  23 +++
> >  3 files changed, 187 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
> >  create mode 100644 include/dt-bindings/reset/bt1-ccu.h
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
> > new file mode 100644
> > index 000000000000..6b1eefdead27
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-axi.yaml
> > @@ -0,0 +1,151 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> Dual license new bindings:
> 
> (GPL-2.0-only OR BSD-2-Clause)

Ok.

> 
> > +#
> > +# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
> > +#
> > +# Baikal-T1 AXI-bus Clocks Control Unit Device Tree Bindings.
> 
> As Stephen said, drop this. You can keep the copyright, just make it 
> line 2.

Ok.

> 
> > +#
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/be,bt1-ccu-axi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Baikal-T1 AXI-bus Clock Control Unit
> > +
> > +maintainers:
> > +  - Serge Semin <fancer.lancer@gmail.com>
> > +
> > +description: |
> > +  Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
> > +  subsystems clocking and resetting. The CCU is connected with an external
> > +  fixed rate oscillator, which signal is transformed into clocks of various
> > +  frequencies and then propagated to either individual IP-blocks or to groups
> > +  of blocks (clock domains). The transformation is done by means of an embedded
> > +  into CCU PLLs and gateable/non-gateable dividers. Each clock domain can be
> > +  also individually reset by using the domain clocks divider configuration
> > +  registers. Baikal-T1 CCU is logically divided into the next components:
> > +  1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
> > +     in general can provide any frequency supported by the CCU PLLs).
> > +  2) PLLs clocks generators (PLLs).
> > +  3) AXI-bus clock dividers (AXI) - described in this bindings file.
> > +  4) System devices reference clock dividers (SYS).
> > +  which are connected with each other as shown on the next figure:
> > +          +---------------+
> > +          | Baikal-T1 CCU |
> > +          |   +----+------|- MIPS P5600 cores
> > +          | +-|PLLs|------|- DDR controller
> > +          | | +----+      |
> > +  +----+  | |  |  |       |
> > +  |XTAL|--|-+  |  | +---+-|
> > +  +----+  | |  |  +-|AXI|-|- AXI-bus
> > +          | |  |    +---+-|
> > +          | |  |          |
> > +          | |  +----+---+-|- APB-bus
> > +          | +-------|SYS|-|- Low-speed Devices
> > +          |         +---+-|- High-speed Devices
> > +          +---------------+
> > +  Each sub-block is represented as a separate dts-node and has an individual
> > +  driver to be bound with.
> > +
> > +  In order to create signals of wide range frequencies the external oscillator
> > +  output is primarily connected to a set of CCU PLLs. Some of PLLs CLKOUT are
> > +  then passed over CCU dividers to create signals required for the target clock
> > +  domain (like AXI-bus consumers). The dividers have the following structure:
> > +          +--------------+
> > +  CLKIN --|->+----+ 1|\  |
> > +  SETCLK--|--|/DIV|->| | |
> > +  CLKDIV--|--|    |  | |-|->CLKLOUT
> > +  LOCK----|--+----+  | | |
> > +          |          |/  |
> > +          |           |  |
> > +  EN------|-----------+  |
> > +  RST-----|--------------|->RSTOUT
> > +          +--------------+
> > +  where CLKIN is the reference clock coming either from a CCU PLL, SETCLK - a
> > +  command to update the output clock in accordance with a set divider,
> > +  CLKDIV - clocks divider, LOCK - a signal of the output clock stabilization,
> > +  EN - enable/disable the divider block, RST/RSTOUT - reset clocks domain
> > +  signal. Depending on the consumer IP-core peculiarities the dividers may lack
> > +  of some functionality depicted on the figure above (like EN,
> > +  CLKDIV/LOCK/SETCLK). In this case the corresponding clock provider just
> > +  doesn't expose either switching functions, or the rate configuration, or
> > +  both of them.
> > +
> > +  The CCU AXI dts-node uses the common clock bindings [1] with no custom
> > +  properties. The list of exported clocks and reset signals can be found in
> > +  the files: 'dt-bindings/clock/bt1-ccu.h' and 'dt-bindings/reset/bt1-ccu.h'.
> > +
> > +  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +
> > +allOf:
> > +  - $ref: /schemas/clock/clock.yaml#
> 
> Drop this, not needed (clock.yaml is applied to every node).

Ok.

> 
> > +
> > +properties:
> > +  compatible:
> > +    const: be,bt1-ccu-axi
> > +
> > +  reg:
> > +    description: AXI-bus CCU dividers sub-block base address.
> 
> Don't really need this.

Ok, but as I said multiple times in others patchsets reply messages,
most likely this property will be removed from the bindings in accordance
with an alternative design of the Baikal-T1 System Controller DT node.
See the next RFC for details: https://lkml.org/lkml/2020/3/22/393

> 
> > +    maxItems: 1
> > +
> > +  "#clock-cells":
> > +    description: |
> > +      Clocks are referenced by the node phandle and an unique identifier
> > +      from 'dt-bindings/clock/bt1-ccu.h'.
> > +    const: 1
> > +
> > +  "#reset-cells":
> > +    description: |
> > +      AXI-bus CCU sub-block provides a reset signal for each clock domain,
> > +      which unique identifiers are in 'dt-bindings/reset/bt1-ccu.h'.
> > +    const: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: CCU SATA PLL output clock.
> > +      - description: CCU PCIe PLL output clock.
> > +      - description: CCU Ethernet PLL output clock.
> > +
> > +  clock-names:
> > +    items:
> > +      - const: sata_clk
> > +      - const: pcie_clk
> > +      - const: eth_clk
> > +
> > +  clock-output-names: true
> > +
> > +  assigned-clocks: true
> > +
> > +  assigned-clock-rates: true
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#clock-cells"
> > +  - clocks
> > +  - clock-names
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/bt1-ccu.h>
> > +
> > +    ccu_axi: ccu_axi@1F04D030 {
> 
> clock-controller@1f04d030

Ok.

> 
> > +      compatible = "be,bt1-ccu-axi";
> > +      reg = <0x1F04D030 0x030>;
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> > +
> > +      clocks = <&ccu_pll CCU_SATA_PLL>,
> > +               <&ccu_pll CCU_PCIE_PLL>,
> > +               <&ccu_pll CCU_ETH_PLL>;
> > +      clock-names = "sata_clk", "pcie_clk", "eth_clk";
> > +
> > +      clock-output-names = "axi_main_clk", "axi_ddr_clk",
> > +                           "axi_sata_clk", "axi_gmac0_clk",
> > +                           "axi_gmac1_clk", "axi_xgmac_clk",
> > +                           "axi_pcie_m_clk", "axi_pcie_s_clk",
> > +                           "axi_usb_clk", "axi_hwa_clk",
> > +                           "axi_sram_clk";
> > +    };
> > +...
> > diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
> > index 86e63162ade0..ebe723c6e0a8 100644
> > --- a/include/dt-bindings/clock/bt1-ccu.h
> > +++ b/include/dt-bindings/clock/bt1-ccu.h
> > @@ -14,4 +14,17 @@
> >  #define CCU_PCIE_PLL			3
> >  #define CCU_ETH_PLL			4
> >  
> > +/* Baikal-T1 AXI-bus CCU Clocks indeces. */
> > +#define CCU_AXI_MAIN_CLK		0
> > +#define CCU_AXI_DDR_CLK			1
> > +#define CCU_AXI_SATA_CLK		2
> > +#define CCU_AXI_GMAC0_CLK		3
> > +#define CCU_AXI_GMAC1_CLK		4
> > +#define CCU_AXI_XGMAC_CLK		5
> > +#define CCU_AXI_PCIE_M_CLK		6
> > +#define CCU_AXI_PCIE_S_CLK		7
> > +#define CCU_AXI_USB_CLK			8
> > +#define CCU_AXI_HWA_CLK			9
> > +#define CCU_AXI_SRAM_CLK		10
> > +
> >  #endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
> > diff --git a/include/dt-bindings/reset/bt1-ccu.h b/include/dt-bindings/reset/bt1-ccu.h
> > new file mode 100644
> > index 000000000000..4de5b6bcd433
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/bt1-ccu.h
> > @@ -0,0 +1,23 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2019 BAIKAL ELECTRONICS, JSC
> > + *
> > + * Baikal-T1 CCU reset indeces.
> > + */
> > +#ifndef __DT_BINDINGS_RESET_BT1_CCU_H
> > +#define __DT_BINDINGS_RESET_BT1_CCU_H
> > +
> > +/* Baikal-T1 AXI-bus CCU Reset indeces. */
> > +#define CCU_AXI_MAIN_RST		0
> > +#define CCU_AXI_DDR_RST			1
> > +#define CCU_AXI_SATA_RST		2
> > +#define CCU_AXI_GMAC0_RST		3
> > +#define CCU_AXI_GMAC1_RST		4
> > +#define CCU_AXI_XGMAC_RST		5
> > +#define CCU_AXI_PCIE_M_RST		6
> > +#define CCU_AXI_PCIE_S_RST		7
> > +#define CCU_AXI_USB_RST			8
> > +#define CCU_AXI_HWA_RST			9
> > +#define CCU_AXI_SRAM_RST		10
> > +
> > +#endif /* __DT_BINDINGS_RESET_BT1_CCU_H */
> > -- 
> > 2.25.1
> > 

Re: [PATCH 3/5] dt-bindings: clk: Add Baikal-T1 System Devices CCU bindings

[Sergey Semin]

On Mon, Mar 09, 2020 at 07:19:12PM -0700, Stephen Boyd wrote:
> Quoting Sergey.Semin@baikalelectronics.ru (2020-03-06 05:00:46)
> > diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml
> > new file mode 100644
> > index 000000000000..aea09fbafc89
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml
> > @@ -0,0 +1,169 @@
> [..]
> > +  assigned-clock-rates: true
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#clock-cells"
> > +  - clocks
> > +  - clock-names
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/bt1-ccu.h>
> > +
> > +    ccu_sys: ccu_sys@1F04D060 {
> 
> Node name should be clock-controller@1f04d060.

Ok.

> 
> Also, binding looks wrong because that address isn't aligned. Most
> likely it's one hardware block that has many different functionalities
> so splitting it up into different regions isn't doing anything besides
> logically splitting up the register space for software benefits.

As I said in RFC: https://lkml.org/lkml/2020/3/22/393 , CCU (Clock
Control Unit) is part of the Baikal-T1 System Control Module (simply
speaking the system controller). In details why I split the system
controller registers space up, I described in the text. Alternatively I
also suggested there to make the CCU nodes being sub-nodes of the System
Control Module DT node. This would better reflect the hardware blocks,
but instead of having an independent registers MMIO in driver I would
have to use the syscon regmap handler.

As I also said in the RFC, in accordance with the Baikal-T1 CCU documentation
CCU is split up into three blocks: PLLs, AXI-bus clocks and System devices
clocks. That's why the CCU driver expects to find three DT nodes:
CCU PLL (be,bt1-ccu-pll), CCU AXI (be,bt1-ccu-axi) and CCU Sys
(be,bt1-ccu-sys).

> 
> > +      compatible = "be,bt1-ccu-sys";
> > +      reg = <0x1F04D060 0x0A0>,
> > +            <0x1F04D150 0x004>;
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> > +
> > +      clocks = <&osc25>,
> > +               <&ccu_pll CCU_SATA_PLL>,
> > +               <&ccu_pll CCU_PCIE_PLL>,
> > +               <&ccu_pll CCU_ETH_PLL>;
> > +      clock-names = "ref_clk", "sata_clk", "pcie_clk",
> > +                    "eth_clk";
> > +
> > +      clock-output-names = "sys_sata_ref_clk", "sys_apb_clk",
> > +                           "sys_gmac0_csr_clk", "sys_gmac0_tx_clk",
> > +                           "sys_gmac0_ptp_clk", "sys_gmac1_csr_clk",
> > +                           "sys_gmac1_tx_clk", "sys_gmac1_ptp_clk",
> > +                           "sys_xgmac_ref_clk", "sys_xgmac_ptp_clk",
> > +                           "sys_usb_clk", "sys_pvt_clk",
> > +                           "sys_hwa_clk", "sys_uart_clk",
> > +                           "sys_spi_clk", "sys_i2c1_clk",
> > +                           "sys_i2c2_clk", "sys_gpio_clk",
> > +                           "sys_timer0_clk", "sys_timer1_clk",
> > +                           "sys_timer2_clk", "sys_wdt_clk";
> > +      };
> > +...
> > diff --git a/include/dt-bindings/reset/bt1-ccu.h b/include/dt-bindings/reset/bt1-ccu.h
> > index 4de5b6bcd433..0bd8fd0edb41 100644
> > --- a/include/dt-bindings/reset/bt1-ccu.h
> > +++ b/include/dt-bindings/reset/bt1-ccu.h
> > @@ -20,4 +20,8 @@
> >  #define CCU_AXI_HWA_RST                        9
> >  #define CCU_AXI_SRAM_RST               10
> >  
> > +/* Baikal-T1 System Devices CCU Reset indeces. */
> 
> indeces is not a word.

Yeah, it was supposed to be "indices". I'll remove this comment anyway.

Regards,
-Sergey

> 
> > +#define CCU_SYS_SATA_REF_RST           0
> > +#define CCU_SYS_APB_RST                        1
> > +

Re: [PATCH 1/5] dt-bindings: clk: Add Baikal-T1 CCU PLLs bindings

[Sergey Semin]

Stephen,

Any back responses on the questions below?

Regards,
-Sergey

On Sun, Apr 05, 2020 at 12:59:25PM +0300, Sergey Semin wrote:
> Hello Stephen,
> 
> Sorry for a delayed response. My answers to your comments are below.
> 
> On Mon, Mar 09, 2020 at 07:02:27PM -0700, Stephen Boyd wrote:
> > Quoting Sergey.Semin@baikalelectronics.ru (2020-03-06 05:00:44)
> > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > 
> > > Baikal-T1 Clocks Control Unit is responsible for transformation of a
> > > signal coming from an external oscillator into clocks of various
> > > frequencies to propagate them then to the corresponding clocks
> > > consumers (either individual IP-blocks or clock domains). In order
> > > to create a set of high-frequency clocks the external signal is
> > > firstly handled by the embedded into CCU PLLs. So the corresponding
> > > dts-node is just a normal clock-provider node with standard set of
> > > properties.
> > > 
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > 
> > SoB chain is backwards. Is Alexey the author? Or Co-developed-by?
> 
> Thanks for noticing this. I'll rearrange the SoB's in v2.
> 
> > 
> > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > Cc: Paul Burton <paulburton@kernel.org>
> > > Cc: Ralf Baechle <ralf@linux-mips.org>
> > > ---
> > >  .../bindings/clock/be,bt1-ccu-pll.yaml        | 139 ++++++++++++++++++
> > >  include/dt-bindings/clock/bt1-ccu.h           |  17 +++
> > >  2 files changed, 156 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> > >  create mode 100644 include/dt-bindings/clock/bt1-ccu.h
> > > 
> > > diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> > > new file mode 100644
> > > index 000000000000..f2e397cc147b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> > > @@ -0,0 +1,139 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
> > > +#
> > > +# Baikal-T1 Clocks Control Unit PLL Device Tree Bindings.
> > > +#
> > 
> > I don't think we need any of these comments besides the license
> > identifier line. Can you dual license this?
> > 
> 
> It's normal to have a copyright here, but in a single-lined form.
> I'll do this in v2 and also dual license the binding file.
> 
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/clock/be,bt1-ccu-pll.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Baikal-T1 Clock Control Unit PLLs
> > > +
> > > +maintainers:
> > > +  - Serge Semin <fancer.lancer@gmail.com>
> > > +
> > > +description: |
> > > +  Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
> > > +  subsystems clocking and resetting. The CCU is connected with an external
> > > +  fixed rate oscillator, which signal is transformed into clocks of various
> > > +  frequencies and then propagated to either individual IP-blocks or to groups
> > > +  of blocks (clock domains). The transformation is done by means of PLLs and
> > > +  gateable/non-gateable dividers embedded into the CCU. It's logically divided
> > > +  into the next components:
> > > +  1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
> > > +     in general can provide any frequency supported by the CCU PLLs).
> > > +  2) PLLs clocks generators (PLLs) - described in this bindings file.
> > > +  3) AXI-bus clock dividers (AXI).
> > > +  4) System devices reference clock dividers (SYS).
> > > +  which are connected with each other as shown on the next figure:
> > 
> > Please add a newline here
> 
> Ok.
> 
> > 
> > > +          +---------------+
> > > +          | Baikal-T1 CCU |
> > > +          |   +----+------|- MIPS P5600 cores
> > > +          | +-|PLLs|------|- DDR controller
> > > +          | | +----+      |
> > > +  +----+  | |  |  |       |
> > > +  |XTAL|--|-+  |  | +---+-|
> > > +  +----+  | |  |  +-|AXI|-|- AXI-bus
> > > +          | |  |    +---+-|
> > > +          | |  |          |
> > > +          | |  +----+---+-|- APB-bus
> > > +          | +-------|SYS|-|- Low-speed Devices
> > > +          |         +---+-|- High-speed Devices
> > > +          +---------------+
> > 
> > And here.
> > 
> 
> Ok
> 
> > > +  Each CCU sub-block is represented as a separate dts-node and has an
> > > +  individual driver to be bound with.
> > > +
> > > +  In order to create signals of wide range frequencies the external oscillator
> > > +  output is primarily connected to a set of CCU PLLs. There are five PLLs
> > > +  to create a clock for the MIPS P5600 cores, the embedded DDR controller,
> > > +  SATA, Ethernet and PCIe domains. The last three domains though named by the
> > > +  biggest system interfaces in fact include nearly all of the rest SoC
> > > +  peripherals. Each of the PLLs is based on True Circuits TSMC CLN28HPM core
> > > +  with an interface wrapper (so called safe PLL' clocks switcher) to simplify
> > > +  the PLL configuration procedure. The PLLs work as depicted on the next
> > > +  diagram:
> > 
> > Same, space out the diagrams.
> > 
> 
> Ok
> 
> > > +      +--------------------------+
> > > +      |                          |
> > > +      +-->+---+    +---+   +---+ |  +---+   0|\
> > > +  CLKF--->|/NF|--->|PFD|...|VCO|-+->|/OD|--->| |
> > > +          +---+ +->+---+   +---+ /->+---+    | |--->CLKOUT
> > > +  CLKOD---------C----------------+          1| |
> > > +       +--------C--------------------------->|/
> > > +       |        |                             ^
> > > +  Rclk-+->+---+ |                             |
> > > +  CLKR--->|/NR|-+                             |
> > > +          +---+                               |
> > > +  BYPASS--------------------------------------+
> > > +  BWADJ--->
> > > +  where Rclk is the reference clock coming  from XTAL, NR - reference clock
> > > +  divider, NF - PLL clock multiplier, OD - VCO output clock divider, CLKOUT -
> > > +  output clock, BWADJ is the PLL bandwidth adjustment parameter. At this moment
> > > +  the binding supports the PLL dividers configuration in accordance with a
> > > +  requested rate, while bypassing and bandwidth adjustment settings can be
> > > +  added in future if it gets to be necessary.
> > > +
> > > +  The PLLs CLKOUT is then either directly connected with the corresponding
> > > +  clocks consumer (like P5600 cores or DDR controller) or passed over a CCU
> > > +  divider to create a signal required for the clock domain.
> > > +
> > > +  The CCU PLL dts-node uses the common clock bindings [1] with no custom
> > > +  parameters. The list of exported clocks can be found in
> > > +  'dt-bindings/clock/bt1-ccu.h'.
> > > +
> > > +  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> > 
> > Don't think we need to mention this binding anymore. But it's good that
> > we know what exported clock ids are.
> > 
> 
> Ok. I'll remove the legacy text binding file mention here and retain the
> reference to the header file with the clock IDs defined. The similar
> thing will be done for the others bindings in the patchset.
> 
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/clock/clock.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: be,bt1-ccu-pll
> > > +
> 
> > > +  reg:
> > > +    description: CCU PLLs sub-block base address.
> > > +    maxItems: 1
> > > +
> 
> Sometime ago I sent a RFC to Rob and you being in Cc there:
> https://lkml.org/lkml/2020/3/22/393
> Simply speaking there are several issues raised in comments to different
> patchsets, which are indirectly connected with the Baikal-T1 System Controller
> DT node design I've initially chosen. In accordance with that I've spread its
> functional blocks into different DT nodes with no reference to being related
> to the System Controller. Clock Control Unit nodes are amongst these blocks.
> Seeing such design caused these issues I suggested an alternative solution
> of having a single System Controller node and multiple functional sub-nodes.
> These sub-nodes will include the Clock Control Unit PLLs, AXI-bus and System
> Device blocks. I thoroughly described the solution in the RFC. So if no
> arguments against it pop up soon in the RFC comments, I'll implement it in
> v2 of this patchset as well. This solution cause the reg-property removal
> from this binding. Instead the drivers shall refer to the parental syscon
> node to get a regmap with CCU registers from it.
> 
> > > +  "#clock-cells":
> > > +    description: |
> > > +      Clocks are referenced by the node phandle and an unique identifier
> > > +      from 'dt-bindings/clock/bt1-ccu.h'.
> > 
> > Don't think we need this description.
> 
> Agreed.
> 
> > 
> > > +    const: 1
> > > +
> > > +  clocks:
> > > +    description: Phandle of CCU External reference clock.
> > > +    maxItems: 1
> > > +
> > > +  clock-names:
> > > +    const: ref_clk
> > 
> > Can we drop _clk? It's redundant.
> 
> I would leave this and "pcie_clk", "sata_clk", "eth_clk" declared in the
> next two bindings as is, since this way they would exactly match the names
> used in the documentation. The same thing is with the clock-output-names
> property values.
> 
> I've seen such names in many other drivers/bindings including the
> bindings in the clock subsystem even submitted rather recently, not to
> mention the names like "aclk", "pclk", etc used all over the dt nodes.
> Are there any requirements in naming the clocks? Should I avoid using the
> '_clk' clock names suffix in accordance with them? If so, please point
> me out to that requirement in docs for future reference.
> 
> Normally If I don't find something in the requirements documented in the kernel,
> I use either a commonly utilized practice seen in other similar drivers, or
> select a solution which seems better to me like providing a better readability
> and code understanding. 
> 
> > 
> > > +
> > > +  clock-output-names: true
> > > +
> > > +  assigned-clocks: true
> > > +
> > > +  assigned-clock-rates: true
> > > +
> > > +additionalProperties: false
> > > +
> 
> I'll also replace these four properties with a single
> "unevaluatedProperties: false". In the framework of other patchset
> review Rob said this property is more suitable in such situations and
> will get to be supported by the dt_binding_check script eventually.
> 
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - "#clock-cells"
> > > +  - clocks
> > > +  - clock-names
> > > +
> > > +examples:
> > > +  - |
> > > +    ccu_pll: ccu_pll@1F04D000 {
> > 
> > Drop the phandle unless it's actually used.
> 
> Do you mean the label definition? If so, Ok. I'll remove it.
> 
> Unit-address will be also lowercased if I don't remove the reg property
> from here. As I said in RFC in accordance with the alternative solution
> this node will be a sub-node of the system controller, which regmap will
> be used instead of the individual reg-property definition. So if the
> reg-property is removed from the node, the unit-address will be also
> discarded from here.
> 
> > 
> > > +      compatible = "be,bt1-ccu-pll";
> > > +      reg = <0x1F04D000 0x028>;
> > 
> > Lowercase hex please. That size is oddly small.
> 
> It's small due to be range being part of the system controller registers
> set. I've briefly described this above and thoroughly - in the RFC.
> Please see the RFC text and send your comments regarding an alternative
> solution there shall you have any.
> 
> Anyway if no comments are received there soon, I'll remove the reg
> property from here. The PLL driver will refer to the parental system
> controller to get the registers regmap handler.
> 
> > 
> > > +      #clock-cells = <1>;
> > > +
> > > +      clocks = <&osc25>;
> > > +      clock-names = "ref_clk";
> > > +
> > > +      clock-output-names = "cpu_pll", "sata_pll", "ddr_pll",
> > > +                           "pcie_pll", "eth_pll";
> > > +    };
> > > +...
> > > diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
> > > new file mode 100644
> > > index 000000000000..86e63162ade0
> > > --- /dev/null
> > > +++ b/include/dt-bindings/clock/bt1-ccu.h
> > > @@ -0,0 +1,17 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Copyright (C) 2019 BAIKAL ELECTRONICS, JSC
> > > + *
> > > + * Baikal-T1 CCU clock indeces.
> > > + */
> > > +#ifndef __DT_BINDINGS_CLOCK_BT1_CCU_H
> > > +#define __DT_BINDINGS_CLOCK_BT1_CCU_H
> > > +
> > > +/* Baikal-T1 CCU PLL indeces. */
> > 
> > Please drop this comment. It's not useful.
> 
> Ok.
> 
> Regards,
> -Sergey
> 
> > 
> > > +#define CCU_CPU_PLL                    0
> > > +#define CCU_SATA_PLL                   1
> > > +#define CCU_DDR_PLL                    2
> > > +#define CCU_PCIE_PLL                   3
> > > +#define CCU_ETH_PLL                    4
> > > +
> > > +#endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
> > > -- 
> > > 2.25.1
> > >

Re: [PATCH 1/5] dt-bindings: clk: Add Baikal-T1 CCU PLLs bindings

[Sergey Semin]

Hello Stephen,
It has been a while since I responded with a few backward questions. Do you
have anything to answer? I am going to send v2 soon, so I need to have those
questions cleared before that.

Regards,
-Sergey

On Sun, Apr 05, 2020 at 12:59:25PM +0300, Sergey Semin wrote:
> Hello Stephen,
> 
> Sorry for a delayed response. My answers to your comments are below.
> 
> On Mon, Mar 09, 2020 at 07:02:27PM -0700, Stephen Boyd wrote:
> > Quoting Sergey.Semin@baikalelectronics.ru (2020-03-06 05:00:44)
> > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > 
> > > Baikal-T1 Clocks Control Unit is responsible for transformation of a
> > > signal coming from an external oscillator into clocks of various
> > > frequencies to propagate them then to the corresponding clocks
> > > consumers (either individual IP-blocks or clock domains). In order
> > > to create a set of high-frequency clocks the external signal is
> > > firstly handled by the embedded into CCU PLLs. So the corresponding
> > > dts-node is just a normal clock-provider node with standard set of
> > > properties.
> > > 
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > 
> > SoB chain is backwards. Is Alexey the author? Or Co-developed-by?
> 
> Thanks for noticing this. I'll rearrange the SoB's in v2.
> 
> > 
> > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > Cc: Paul Burton <paulburton@kernel.org>
> > > Cc: Ralf Baechle <ralf@linux-mips.org>
> > > ---
> > >  .../bindings/clock/be,bt1-ccu-pll.yaml        | 139 ++++++++++++++++++
> > >  include/dt-bindings/clock/bt1-ccu.h           |  17 +++
> > >  2 files changed, 156 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> > >  create mode 100644 include/dt-bindings/clock/bt1-ccu.h
> > > 
> > > diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> > > new file mode 100644
> > > index 000000000000..f2e397cc147b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-pll.yaml
> > > @@ -0,0 +1,139 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Copyright (C) 2019 - 2020 BAIKAL ELECTRONICS, JSC
> > > +#
> > > +# Baikal-T1 Clocks Control Unit PLL Device Tree Bindings.
> > > +#
> > 
> > I don't think we need any of these comments besides the license
> > identifier line. Can you dual license this?
> > 
> 
> It's normal to have a copyright here, but in a single-lined form.
> I'll do this in v2 and also dual license the binding file.
> 
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/clock/be,bt1-ccu-pll.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Baikal-T1 Clock Control Unit PLLs
> > > +
> > > +maintainers:
> > > +  - Serge Semin <fancer.lancer@gmail.com>
> > > +
> > > +description: |
> > > +  Clocks Control Unit is the core of Baikal-T1 SoC responsible for the chip
> > > +  subsystems clocking and resetting. The CCU is connected with an external
> > > +  fixed rate oscillator, which signal is transformed into clocks of various
> > > +  frequencies and then propagated to either individual IP-blocks or to groups
> > > +  of blocks (clock domains). The transformation is done by means of PLLs and
> > > +  gateable/non-gateable dividers embedded into the CCU. It's logically divided
> > > +  into the next components:
> > > +  1) External oscillator (normally XTAL's 25 MHz crystal oscillator, but
> > > +     in general can provide any frequency supported by the CCU PLLs).
> > > +  2) PLLs clocks generators (PLLs) - described in this bindings file.
> > > +  3) AXI-bus clock dividers (AXI).
> > > +  4) System devices reference clock dividers (SYS).
> > > +  which are connected with each other as shown on the next figure:
> > 
> > Please add a newline here
> 
> Ok.
> 
> > 
> > > +          +---------------+
> > > +          | Baikal-T1 CCU |
> > > +          |   +----+------|- MIPS P5600 cores
> > > +          | +-|PLLs|------|- DDR controller
> > > +          | | +----+      |
> > > +  +----+  | |  |  |       |
> > > +  |XTAL|--|-+  |  | +---+-|
> > > +  +----+  | |  |  +-|AXI|-|- AXI-bus
> > > +          | |  |    +---+-|
> > > +          | |  |          |
> > > +          | |  +----+---+-|- APB-bus
> > > +          | +-------|SYS|-|- Low-speed Devices
> > > +          |         +---+-|- High-speed Devices
> > > +          +---------------+
> > 
> > And here.
> > 
> 
> Ok
> 
> > > +  Each CCU sub-block is represented as a separate dts-node and has an
> > > +  individual driver to be bound with.
> > > +
> > > +  In order to create signals of wide range frequencies the external oscillator
> > > +  output is primarily connected to a set of CCU PLLs. There are five PLLs
> > > +  to create a clock for the MIPS P5600 cores, the embedded DDR controller,
> > > +  SATA, Ethernet and PCIe domains. The last three domains though named by the
> > > +  biggest system interfaces in fact include nearly all of the rest SoC
> > > +  peripherals. Each of the PLLs is based on True Circuits TSMC CLN28HPM core
> > > +  with an interface wrapper (so called safe PLL' clocks switcher) to simplify
> > > +  the PLL configuration procedure. The PLLs work as depicted on the next
> > > +  diagram:
> > 
> > Same, space out the diagrams.
> > 
> 
> Ok
> 
> > > +      +--------------------------+
> > > +      |                          |
> > > +      +-->+---+    +---+   +---+ |  +---+   0|\
> > > +  CLKF--->|/NF|--->|PFD|...|VCO|-+->|/OD|--->| |
> > > +          +---+ +->+---+   +---+ /->+---+    | |--->CLKOUT
> > > +  CLKOD---------C----------------+          1| |
> > > +       +--------C--------------------------->|/
> > > +       |        |                             ^
> > > +  Rclk-+->+---+ |                             |
> > > +  CLKR--->|/NR|-+                             |
> > > +          +---+                               |
> > > +  BYPASS--------------------------------------+
> > > +  BWADJ--->
> > > +  where Rclk is the reference clock coming  from XTAL, NR - reference clock
> > > +  divider, NF - PLL clock multiplier, OD - VCO output clock divider, CLKOUT -
> > > +  output clock, BWADJ is the PLL bandwidth adjustment parameter. At this moment
> > > +  the binding supports the PLL dividers configuration in accordance with a
> > > +  requested rate, while bypassing and bandwidth adjustment settings can be
> > > +  added in future if it gets to be necessary.
> > > +
> > > +  The PLLs CLKOUT is then either directly connected with the corresponding
> > > +  clocks consumer (like P5600 cores or DDR controller) or passed over a CCU
> > > +  divider to create a signal required for the clock domain.
> > > +
> > > +  The CCU PLL dts-node uses the common clock bindings [1] with no custom
> > > +  parameters. The list of exported clocks can be found in
> > > +  'dt-bindings/clock/bt1-ccu.h'.
> > > +
> > > +  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> > 
> > Don't think we need to mention this binding anymore. But it's good that
> > we know what exported clock ids are.
> > 
> 
> Ok. I'll remove the legacy text binding file mention here and retain the
> reference to the header file with the clock IDs defined. The similar
> thing will be done for the others bindings in the patchset.
> 
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/clock/clock.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: be,bt1-ccu-pll
> > > +
> 
> > > +  reg:
> > > +    description: CCU PLLs sub-block base address.
> > > +    maxItems: 1
> > > +
> 
> Sometime ago I sent a RFC to Rob and you being in Cc there:
> https://lkml.org/lkml/2020/3/22/393
> Simply speaking there are several issues raised in comments to different
> patchsets, which are indirectly connected with the Baikal-T1 System Controller
> DT node design I've initially chosen. In accordance with that I've spread its
> functional blocks into different DT nodes with no reference to being related
> to the System Controller. Clock Control Unit nodes are amongst these blocks.
> Seeing such design caused these issues I suggested an alternative solution
> of having a single System Controller node and multiple functional sub-nodes.
> These sub-nodes will include the Clock Control Unit PLLs, AXI-bus and System
> Device blocks. I thoroughly described the solution in the RFC. So if no
> arguments against it pop up soon in the RFC comments, I'll implement it in
> v2 of this patchset as well. This solution cause the reg-property removal
> from this binding. Instead the drivers shall refer to the parental syscon
> node to get a regmap with CCU registers from it.
> 
> > > +  "#clock-cells":
> > > +    description: |
> > > +      Clocks are referenced by the node phandle and an unique identifier
> > > +      from 'dt-bindings/clock/bt1-ccu.h'.
> > 
> > Don't think we need this description.
> 
> Agreed.
> 
> > 
> > > +    const: 1
> > > +
> > > +  clocks:
> > > +    description: Phandle of CCU External reference clock.
> > > +    maxItems: 1
> > > +
> > > +  clock-names:
> > > +    const: ref_clk
> > 
> > Can we drop _clk? It's redundant.
> 
> I would leave this and "pcie_clk", "sata_clk", "eth_clk" declared in the
> next two bindings as is, since this way they would exactly match the names
> used in the documentation. The same thing is with the clock-output-names
> property values.
> 
> I've seen such names in many other drivers/bindings including the
> bindings in the clock subsystem even submitted rather recently, not to
> mention the names like "aclk", "pclk", etc used all over the dt nodes.
> Are there any requirements in naming the clocks? Should I avoid using the
> '_clk' clock names suffix in accordance with them? If so, please point
> me out to that requirement in docs for future reference.
> 
> Normally If I don't find something in the requirements documented in the kernel,
> I use either a commonly utilized practice seen in other similar drivers, or
> select a solution which seems better to me like providing a better readability
> and code understanding. 
> 
> > 
> > > +
> > > +  clock-output-names: true
> > > +
> > > +  assigned-clocks: true
> > > +
> > > +  assigned-clock-rates: true
> > > +
> > > +additionalProperties: false
> > > +
> 
> I'll also replace these four properties with a single
> "unevaluatedProperties: false". In the framework of other patchset
> review Rob said this property is more suitable in such situations and
> will get to be supported by the dt_binding_check script eventually.
> 
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - "#clock-cells"
> > > +  - clocks
> > > +  - clock-names
> > > +
> > > +examples:
> > > +  - |
> > > +    ccu_pll: ccu_pll@1F04D000 {
> > 
> > Drop the phandle unless it's actually used.
> 
> Do you mean the label definition? If so, Ok. I'll remove it.
> 
> Unit-address will be also lowercased if I don't remove the reg property
> from here. As I said in RFC in accordance with the alternative solution
> this node will be a sub-node of the system controller, which regmap will
> be used instead of the individual reg-property definition. So if the
> reg-property is removed from the node, the unit-address will be also
> discarded from here.
> 
> > 
> > > +      compatible = "be,bt1-ccu-pll";
> > > +      reg = <0x1F04D000 0x028>;
> > 
> > Lowercase hex please. That size is oddly small.
> 
> It's small due to be range being part of the system controller registers
> set. I've briefly described this above and thoroughly - in the RFC.
> Please see the RFC text and send your comments regarding an alternative
> solution there shall you have any.
> 
> Anyway if no comments are received there soon, I'll remove the reg
> property from here. The PLL driver will refer to the parental system
> controller to get the registers regmap handler.
> 
> > 
> > > +      #clock-cells = <1>;
> > > +
> > > +      clocks = <&osc25>;
> > > +      clock-names = "ref_clk";
> > > +
> > > +      clock-output-names = "cpu_pll", "sata_pll", "ddr_pll",
> > > +                           "pcie_pll", "eth_pll";
> > > +    };
> > > +...
> > > diff --git a/include/dt-bindings/clock/bt1-ccu.h b/include/dt-bindings/clock/bt1-ccu.h
> > > new file mode 100644
> > > index 000000000000..86e63162ade0
> > > --- /dev/null
> > > +++ b/include/dt-bindings/clock/bt1-ccu.h
> > > @@ -0,0 +1,17 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Copyright (C) 2019 BAIKAL ELECTRONICS, JSC
> > > + *
> > > + * Baikal-T1 CCU clock indeces.
> > > + */
> > > +#ifndef __DT_BINDINGS_CLOCK_BT1_CCU_H
> > > +#define __DT_BINDINGS_CLOCK_BT1_CCU_H
> > > +
> > > +/* Baikal-T1 CCU PLL indeces. */
> > 
> > Please drop this comment. It's not useful.
> 
> Ok.
> 
> Regards,
> -Sergey
> 
> > 
> > > +#define CCU_CPU_PLL                    0
> > > +#define CCU_SATA_PLL                   1
> > > +#define CCU_DDR_PLL                    2
> > > +#define CCU_PCIE_PLL                   3
> > > +#define CCU_ETH_PLL                    4
> > > +
> > > +#endif /* __DT_BINDINGS_CLOCK_BT1_CCU_H */
> > > -- 
> > > 2.25.1
> > >