Re: [PATCH 2/3] clk: shmobile: Add MSTP clock support

Hi Kumar,

Thank you for the review.

On Tuesday 29 October 2013 18:36:06 Kumar Gala wrote:

On Oct 29, 2013, at 9:55 AM, Laurent Pinchart wrote:

quoted

MSTP clocks are gate clocks controlled through a register that handles
up to 32 clocks. The register is often sparsely populated.

Those clocks are found on Renesas ARM SoCs.

Signed-off-by: Laurent Pinchart
[off-list ref]
---
.../bindings/clock/renesas,cpg-mstp-clocks.txt     |  47 +++++
drivers/clk/shmobile/Makefile                      |   1 +
drivers/clk/shmobile/clk-mstp.c                    | 229 +++++++++++++++++
include/dt-bindings/clock/r8a7790-clock.h          |  56 +++++
4 files changed, 333 insertions(+)
create mode 100644
Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
create mode 100644 drivers/clk/shmobile/clk-mstp.c
create mode 100644 include/dt-bindings/clock/r8a7790-clock.h

diff --git
a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt new
file mode 100644
index 0000000..b3a1ce0

--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt

@@ -0,0 +1,47 @@
+* Renesas R8A7790 MSTP Clocks

can we spell out MSTP once in the heading?

Sure thing. It stands for Module Stop.

quoted

+
+The CPG can gate SoC device clocks. The gates are organized in groups of
up to
+32 gates.
+
+This device tree binding describes a single 32 gate clocks group per
node.
+Clocks are referenced by user nodes by the MSTP node phandle and the
clock
+index in the group, from 0 to 31.
+
+Required Properties:
+
+  - compatible: Must be one of the following
+    - "renesas,r8a7790-mstp-clocks" for R8A7790 (R-Car H2) MSTP gate
clocks
+    - "renesas,cpg-mstp-clock" for generic MSTP gate clocks
+  - reg: Base address and length of the memory resource used by the MSTP
+    clocks
+  - clocks: Reference to the parent clocks
+  - #clock-cells: Must be 1
+  - clock-output-names: The name of the clocks as free-form strings
+  - renesas,indices: Index of the gate clocks (0 to 31)

Index of the gate clock into what?

Into the 32 gates clock group. Groups have 32 entries with a 32-bit register 
that controls 32 gate clocks. The groups (and thus registers) are sparsly 
populated, this property lists the indices for the register bits corresponding 
to the clocks.

Would

  - renesas,indices: Indices of the gate clocks into the group (0 to 31)

be explicit enough ?

quoted

+
+The clocks, clock-output-names and renesas,indices properties contain one
+entry per gate. The MSTP groups are sparsely populated. Unimplemented
gates

per gate clock. (?)

I'll change that.

quoted

+must not be declared.
+
+
+Example
+-------
+
+	#include <dt-bindings/clock/r8a7790-clock.h>
+
+	mstp3_clks: mstp3_clks {

mstp3_clks@e615013c I suppose.

quoted

+		compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-

clocks";

quoted

+		reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
+		clocks = <&cp_clk>, <&mmc1_clk>, <&sd3_clk>, <&sd2_clk>,
+			 <&cpg_clocks R8A7790_CLK_SD1>, <&cpg_clocks 

R8A7790_CLK_SD0>,

quoted

+			 <&mmc0_clk>;
+		#clock-cells = <1>;
+		clock-output-names =
+			"tpu0", "mmcif1", "sdhi3", "sdhi2",
+			 "sdhi1", "sdhi0", "mmcif0";
+		renesas,clock-indices = <
+			R8A7790_CLK_TPU0 R8A7790_CLK_MMCIF1 R8A7790_CLK_SDHI3
+			R8A7790_CLK_SDHI2 R8A7790_CLK_SDHI1 R8A7790_CLK_SDHI0
+			R8A7790_CLK_MMCIF0
+		>;
+	};

-- 
Regards,

Laurent Pinchart